[Routing] Implement bug fixes and enhancements

CHANGELOG-2.1.md

@@ -363,9 +363,11 @@ To get the diff between two versions, go to https://github.com/symfony/symfony/c
  * added a TraceableUrlMatcher
  * added the possibility to define options, default values and requirements for placeholders in prefix, including imported routes
  * added RouterInterface::getRouteCollection
- * [BC BREAK] the UrlMatcher urldecodes the route parameters only once, they were
+ * [BC BREAK] the UrlMatcher urldecodes the route parameters only once, they were decoded twice before.
-   decoded twice before. Note that the `urldecode()` calls have been change for a
+   Note that the `urldecode()` calls have been changed for a single `rawurldecode()` in order to support `+` for input paths.
-   single `rawurldecode()` in order to support `+` for input paths.
+ * added RouteCollection::getRoot method to retrieve the root of a RouteCollection tree
+ * [BC BREAK] made RouteCollection::setParent private which could not have been used anyway without creating inconsistencies
+ * [BC BREAK] RouteCollection::remove also removes a route from parent collections (not only from its children)
 
 ### Security
 

src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

@@ -18,6 +18,7 @@ use Symfony\Component\Routing\RouteCollection;
  * PhpMatcherDumper creates a PHP class able to match URLs for a given set of routes.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ * @author Tobias Schultze <http://tobion.de>
  */
 class PhpMatcherDumper extends MatcherDumper
 {
@@ -42,23 +43,49 @@ class PhpMatcherDumper extends MatcherDumper
 
         // trailing slash support is only enabled if we know how to redirect the user
         $interfaces = class_implements($options['base_class']);
-        $supportsRedirections = isset($interfaces['Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface']);
+        $supportsRedirections = isset($interfaces['Symfony\\Component\\Routing\\Matcher\\RedirectableUrlMatcherInterface']);
-
-        return
-            $this->startClass($options['class'], $options['base_class']).
-            $this->addConstructor().
-            $this->addMatcher($supportsRedirections).
-            $this->endClass()
-        ;
-    }
-
-    private function addMatcher($supportsRedirections)
-    {
-        // we need to deep clone the routes as we will modify the structure to optimize the dump
-        $code = rtrim($this->compileRoutes(clone $this->getRoutes(), $supportsRedirections), "\n");
 
         return <<<EOF
+<?php
 
+use Symfony\Component\Routing\Exception\MethodNotAllowedException;
+use Symfony\Component\Routing\Exception\ResourceNotFoundException;
+use Symfony\Component\Routing\RequestContext;
+
+/**
+ * {$options['class']}
+ *
+ * This class has been auto-generated
+ * by the Symfony Routing Component.
+ */
+class {$options['class']} extends {$options['base_class']}
+{
+    /**
+     * Constructor.
+     */
+    public function __construct(RequestContext \$context)
+    {
+        \$this->context = \$context;
+    }
+
+{$this->generateMatchMethod($supportsRedirections)}
+}
+
+EOF;
+    }
+
+    /**
+     * Generates the code for the match method implementing UrlMatcherInterface.
+     *
+     * @param Boolean $supportsRedirections Whether redirections are supported by the base class
+     *
+     * @return string Match method as PHP code
+     */
+    private function generateMatchMethod($supportsRedirections)
+    {
+        $code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections), "\n");
+
+        return <<<EOF
     public function match(\$pathinfo)
     {
         \$allow = array();
@@ -68,78 +95,83 @@ $code
 
         throw 0 < count(\$allow) ? new MethodNotAllowedException(array_unique(\$allow)) : new ResourceNotFoundException();
     }
-
 EOF;
     }
 
+    /**
+     * Counts the number of routes as direct child of the RouteCollection.
+     *
+     * @param RouteCollection $routes A RouteCollection instance
+     *
+     * @return integer Number of Routes
+     */
+    private function countDirectChildRoutes(RouteCollection $routes)
+    {
+        $count = 0;
+        foreach ($routes as $route) {
+            if ($route instanceof Route) {
+                $count++;
+            }
+        }
+
+        return $count;
+    }
+
+    /**
+     * Generates PHP code recursively to match a RouteCollection with all child routes and child collections.
+     *
+     * @param RouteCollection $routes               A RouteCollection instance
+     * @param Boolean         $supportsRedirections Whether redirections are supported by the base class
+     * @param string|null     $parentPrefix         The prefix of the parent collection used to optimize the code
+     *
+     * @return string PHP code
+     */
     private function compileRoutes(RouteCollection $routes, $supportsRedirections, $parentPrefix = null)
     {
         $code = '';
 
-        $routeIterator = $routes->getIterator();
+        $prefix = $routes->getPrefix();
-        $keys = array_keys($routeIterator->getArrayCopy());
+        $countDirectChildRoutes = $this->countDirectChildRoutes($routes);
-        $keysCount = count($keys);
+        $countAllChildRoutes = count($routes->all());
+        // Can the matching be optimized by wrapping it with the prefix condition
+        // - no need to optimize if current prefix is the same as the parent prefix
+        // - if $countDirectChildRoutes === 0, the sub-collections can do their own optimizations (in case there are any)
+        // - it's not worth wrapping a single child route
+        // - prefixes with variables cannot be optimized because routes within the collection might have different requirements for the same variable
+        $optimizable = '' !== $prefix && $prefix !== $parentPrefix && $countDirectChildRoutes > 0 && $countAllChildRoutes > 1 && false === strpos($prefix, '{');
+        if ($optimizable) {
+            $code .= sprintf("    if (0 === strpos(\$pathinfo, %s)) {\n", var_export($prefix, true));
+        }
 
-        $i = 0;
+        foreach ($routes as $name => $route) {
-        foreach ($routeIterator as $name => $route) {
+            if ($route instanceof Route) {
-            $i++;
+                // a single route in a sub-collection is not wrapped so it should do its own optimization in ->compileRoute with $parentPrefix = null
-
+                $code .= $this->compileRoute($route, $name, $supportsRedirections, 1 === $countAllChildRoutes ? null : $prefix)."\n";
-            if ($route instanceof RouteCollection) {
+            } elseif ($countAllChildRoutes - $countDirectChildRoutes > 0) { // we can stop iterating recursively if we already know there are no more routes
-                $prefix = $route->getPrefix();
+                $code .= $this->compileRoutes($route, $supportsRedirections, $prefix);
-                $optimizable = $prefix && count($route->all()) > 1 && false === strpos($route->getPrefix(), '{');
-                $optimized = false;
-
-                if ($optimizable) {
-                    for ($j = $i; $j < $keysCount; $j++) {
-                        if (null === $keys[$j]) {
-                            continue;
-                        }
-
-                        $testRoute = $routeIterator->offsetGet($keys[$j]);
-                        $isCollection = ($testRoute instanceof RouteCollection);
-
-                        $testPrefix = $isCollection ? $testRoute->getPrefix() : $testRoute->getPattern();
-
-                        if (0 === strpos($testPrefix, $prefix)) {
-                            $routeIterator->offsetUnset($keys[$j]);
-
-                            if ($isCollection) {
-                                $route->addCollection($testRoute);
-                            } else {
-                                $route->add($keys[$j], $testRoute);
-                            }
-
-                            $i++;
-                            $keys[$j] = null;
-                        }
-                    }
-
-                    if ($prefix !== $parentPrefix) {
-                        $code .= sprintf("        if (0 === strpos(\$pathinfo, %s)) {\n", var_export($prefix, true));
-                        $optimized = true;
-                    }
-                }
-
-                if ($optimized) {
-                    foreach (explode("\n", $this->compileRoutes($route, $supportsRedirections, $prefix)) as $line) {
-                        if ('' !== $line) {
-                            $code .= '    '; // apply extra indention
-                        }
-                        $code .= $line."\n";
-                    }
-                    $code = substr($code, 0, -2); // remove redundant last two line breaks
-                    $code .= "        }\n\n";
-                } else {
-                    $code .= $this->compileRoutes($route, $supportsRedirections, $prefix);
-                }
-            } else {
-                $code .= $this->compileRoute($route, $name, $supportsRedirections, $parentPrefix)."\n";
             }
         }
 
+        if ($optimizable) {
+            $code .= "    }\n\n";
+            // apply extra indention at each line (except empty ones)
+            $code = preg_replace('/^.{2,}$/m', '    $0', $code);
+        }
+
         return $code;
     }
 
+
+    /**
+     * Compiles a single Route to PHP code used to match it against the path info.
+     *
+     * @param Route       $routes               A Route instance
+     * @param string      $name                 The name of the Route
+     * @param Boolean     $supportsRedirections Whether redirections are supported by the base class
+     * @param string|null $parentPrefix         The prefix of the parent collection used to optimize the code
+     *
+     * @return string PHP code
+     */
     private function compileRoute(Route $route, $name, $supportsRedirections, $parentPrefix = null)
     {
         $code = '';
@@ -167,7 +199,7 @@ EOF;
                 $conditions[] = sprintf("\$pathinfo === %s", var_export(str_replace('\\', '', $m['url']), true));
             }
         } else {
-            if ($compiledRoute->getStaticPrefix() && $compiledRoute->getStaticPrefix() != $parentPrefix) {
+            if ($compiledRoute->getStaticPrefix() && $compiledRoute->getStaticPrefix() !== $parentPrefix) {
                 $conditions[] = sprintf("0 === strpos(\$pathinfo, %s)", var_export($compiledRoute->getStaticPrefix(), true));
             }
 
@@ -254,47 +286,4 @@ EOF;
 
         return $code;
     }
-
-    private function startClass($class, $baseClass)
-    {
-        return <<<EOF
-<?php
-
-use Symfony\Component\Routing\Exception\MethodNotAllowedException;
-use Symfony\Component\Routing\Exception\ResourceNotFoundException;
-use Symfony\Component\Routing\RequestContext;
-
-/**
- * $class
- *
- * This class has been auto-generated
- * by the Symfony Routing Component.
- */
-class $class extends $baseClass
-{
-
-EOF;
-    }
-
-    private function addConstructor()
-    {
-        return <<<EOF
-    /**
-     * Constructor.
-     */
-    public function __construct(RequestContext \$context)
-    {
-        \$this->context = \$context;
-    }
-
-EOF;
-    }
-
-    private function endClass()
-    {
-        return <<<EOF
-}
-
-EOF;
-    }
 }

src/Symfony/Component/Routing/Route.php

@@ -79,10 +79,12 @@ class Route
         $this->pattern = trim($pattern);
 
         // a route must start with a slash
-        if (empty($this->pattern) || '/' !== $this->pattern[0]) {
+        if ('' === $this->pattern || '/' !== $this->pattern[0]) {
             $this->pattern = '/'.$this->pattern;
         }
 
+        $this->compiled = null;
+
         return $this;
     }
 
@@ -128,6 +130,7 @@ class Route
         foreach ($options as $name => $option) {
             $this->options[(string) $name] = $option;
         }
+        $this->compiled = null;
 
         return $this;
     }
@@ -147,6 +150,7 @@ class Route
     public function setOption($name, $value)
     {
         $this->options[$name] = $value;
+        $this->compiled = null;
 
         return $this;
     }
@@ -203,6 +207,7 @@ class Route
         foreach ($defaults as $name => $default) {
             $this->defaults[(string) $name] = $default;
         }
+        $this->compiled = null;
 
         return $this;
     }
@@ -244,6 +249,7 @@ class Route
     public function setDefault($name, $default)
     {
         $this->defaults[(string) $name] = $default;
+        $this->compiled = null;
 
         return $this;
     }
@@ -288,6 +294,7 @@ class Route
         foreach ($requirements as $key => $regex) {
             $this->requirements[$key] = $this->sanitizeRequirement($key, $regex);
         }
+        $this->compiled = null;
 
         return $this;
     }
@@ -317,6 +324,7 @@ class Route
     public function setRequirement($key, $regex)
     {
         $this->requirements[$key] = $this->sanitizeRequirement($key, $regex);
+        $this->compiled = null;
 
         return $this;
     }
@@ -343,15 +351,19 @@ class Route
 
     private function sanitizeRequirement($key, $regex)
     {
-        if (is_array($regex)) {
+        if (!is_string($regex)) {
-            throw new \InvalidArgumentException(sprintf('Routing requirements must be a string, array given for "%s"', $key));
+            throw new \InvalidArgumentException(sprintf('Routing requirement for "%s" must be a string', $key));
         }
 
-        if ('^' == $regex[0]) {
+        if ('' === $regex) {
+            throw new \InvalidArgumentException(sprintf('Routing requirement for "%s" cannot be empty', $key));
+        }
+
+        if ('^' === $regex[0]) {
             $regex = substr($regex, 1);
         }
 
-        if ('$' == substr($regex, -1)) {
+        if ('$' === substr($regex, -1)) {
             $regex = substr($regex, 0, -1);
         }
 

src/Symfony/Component/Routing/RouteCollection.php

@@ -14,12 +14,13 @@ namespace Symfony\Component\Routing;
 use Symfony\Component\Config\Resource\ResourceInterface;
 
 /**
- * A RouteCollection represents a set of Route instances.
+ * A RouteCollection represents a set of Route instances as a tree structure.
  *
  * When adding a route, it overrides existing routes with the
- * same name defined in theinstance or its children and parents.
+ * same name defined in the instance or its children and parents.
  *
  * @author Fabien Potencier <fabien@symfony.com>
+ * @author Tobias Schultze <http://tobion.de>
  *
  * @api
  */
@@ -55,7 +56,7 @@ class RouteCollection implements \IteratorAggregate
     /**
      * Gets the parent RouteCollection.
      *
-     * @return RouteCollection The parent RouteCollection
+     * @return RouteCollection|null The parent RouteCollection or null when it's the root
      */
     public function getParent()
     {
@@ -63,13 +64,18 @@ class RouteCollection implements \IteratorAggregate
     }
 
     /**
-     * Sets the parent RouteCollection.
+     * Gets the root RouteCollection of the tree.
      *
-     * @param RouteCollection $parent The parent RouteCollection
+     * @return RouteCollection The root RouteCollection
      */
-    public function setParent(RouteCollection $parent)
+    public function getRoot()
     {
-        $this->parent = $parent;
+        $parent = $this;
+        while ($parent->getParent()) {
+            $parent = $parent->getParent();
+        }
+
+        return $parent;
     }
 
     /**
@@ -98,20 +104,13 @@ class RouteCollection implements \IteratorAggregate
             throw new \InvalidArgumentException(sprintf('The provided route name "%s" contains non valid characters. A route name must only contain digits (0-9), letters (a-z and A-Z), underscores (_) and dots (.).', $name));
         }
 
-        $parent = $this;
+        $this->remove($name);
-        while ($parent->getParent()) {
-            $parent = $parent->getParent();
-        }
-
-        if ($parent) {
-            $parent->remove($name);
-        }
 
         $this->routes[$name] = $route;
     }
 
     /**
-     * Returns the array of routes.
+     * Returns all routes in this collection and its children.
      *
      * @return array An array of routes
      */
@@ -130,45 +129,39 @@ class RouteCollection implements \IteratorAggregate
     }
 
     /**
-     * Gets a route by name.
+     * Gets a route by name defined in this collection or its children.
      *
-     * @param  string $name  The route name
+     * @param string $name The route name
      *
-     * @return Route  $route A Route instance
+     * @return Route|null A Route instance or null when not found
      */
     public function get($name)
     {
-        // get the latest defined route
+        if (isset($this->routes[$name])) {
-        foreach (array_reverse($this->routes) as $routes) {
+            return $this->routes[$name] instanceof RouteCollection ? null : $this->routes[$name];
-            if (!$routes instanceof RouteCollection) {
+        }
-                continue;
-            }
 
-            if (null !== $route = $routes->get($name)) {
+        foreach ($this->routes as $routes) {
+            if ($routes instanceof RouteCollection && null !== $route = $routes->get($name)) {
                 return $route;
             }
         }
 
-        if (isset($this->routes[$name])) {
+        return null;
-            return $this->routes[$name];
-        }
     }
 
     /**
-     * Removes a route by name.
+     * Removes a route or an array of routes by name from all connected
+     * collections (this instance and all parents and children).
      *
-     * @param string $name The route name
+     * @param string|array $name The route name or an array of route names
      */
     public function remove($name)
     {
-        if (isset($this->routes[$name])) {
+        $root = $this->getRoot();
-            unset($this->routes[$name]);
-        }
 
-        foreach ($this->routes as $routes) {
+        foreach ((array) $name as $n) {
-            if ($routes instanceof RouteCollection) {
+            $root->removeRecursively($n);
-                $routes->remove($name);
-            }
         }
     }
 
@@ -181,18 +174,25 @@ class RouteCollection implements \IteratorAggregate
      * @param array           $requirements An array of requirements
      * @param array           $options      An array of options
      *
+     * @throws \InvalidArgumentException When the RouteCollection already exists in the tree
+     *
      * @api
      */
     public function addCollection(RouteCollection $collection, $prefix = '', $defaults = array(), $requirements = array(), $options = array())
     {
-        $collection->setParent($this);
+        // prevent infinite loops by recursive referencing
-        $collection->addPrefix($prefix, $defaults, $requirements, $options);
+        $root = $this->getRoot();
-
+        if ($root === $collection || $root->hasCollection($collection)) {
-        // remove all routes with the same name in all existing collections
+            throw new \InvalidArgumentException('The RouteCollection already exists in the tree.');
-        foreach (array_keys($collection->all()) as $name) {
-            $this->remove($name);
         }
 
+        // remove all routes with the same names in all existing collections
+        $this->remove(array_keys($collection->all()));
+
+        $collection->setParent($this);
+        // the sub-collection must have the prefix of the parent (current instance) prepended because it does not
+        // necessarily already have it applied (depending on the order RouteCollections are added to each other)
+        $collection->addPrefix($this->getPrefix() . $prefix, $defaults, $requirements, $options);
         $this->routes[] = $collection;
     }
 
@@ -211,14 +211,18 @@ class RouteCollection implements \IteratorAggregate
         // a prefix must not end with a slash
         $prefix = rtrim($prefix, '/');
 
+        if ('' === $prefix && empty($defaults) && empty($requirements) && empty($options)) {
+            return;
+        }
+
         // a prefix must start with a slash
-        if ($prefix && '/' !== $prefix[0]) {
+        if ('' !== $prefix && '/' !== $prefix[0]) {
             $prefix = '/'.$prefix;
         }
 
         $this->prefix = $prefix.$this->prefix;
 
-        foreach ($this->routes as $name => $route) {
+        foreach ($this->routes as $route) {
             if ($route instanceof RouteCollection) {
                 $route->addPrefix($prefix, $defaults, $requirements, $options);
             } else {
@@ -230,6 +234,11 @@ class RouteCollection implements \IteratorAggregate
         }
     }
 
+    /**
+     * Returns the prefix that may contain placeholders.
+     *
+     * @return string The prefix
+     */
     public function getPrefix()
     {
         return $this->prefix;
@@ -261,4 +270,60 @@ class RouteCollection implements \IteratorAggregate
     {
         $this->resources[] = $resource;
     }
+
+    /**
+     * Sets the parent RouteCollection. It's only used internally from one RouteCollection
+     * to another. It makes no sense to be available as part of the public API.
+     *
+     * @param RouteCollection $parent The parent RouteCollection
+     */
+    private function setParent(RouteCollection $parent)
+    {
+        $this->parent = $parent;
+    }
+
+    /**
+     * Removes a route by name from this collection and its children recursively.
+     *
+     * @param string $name The route name
+     *
+     * @return Boolean true when found
+     */
+    private function removeRecursively($name)
+    {
+        // It is ensured by the adders (->add and ->addCollection) that there can
+        // only be one route per name in all connected collections. So we can stop
+        // interating recursively on the first hit.
+        if (isset($this->routes[$name])) {
+            unset($this->routes[$name]);
+
+            return true;
+        }
+
+        foreach ($this->routes as $routes) {
+            if ($routes instanceof RouteCollection && $routes->removeRecursively($name)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    /**
+     * Checks whether the given RouteCollection is already set in any child of the current instance.
+     *
+     * @param RouteCollection $collection A RouteCollection instance
+     *
+     * @return Boolean
+     */
+    private function hasCollection(RouteCollection $collection)
+    {
+        foreach ($this->routes as $routes) {
+            if ($routes === $collection || $routes instanceof RouteCollection && $routes->hasCollection($collection)) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }

src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php

@@ -125,6 +125,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
                     return $matches;
                 }
 
+            }
+
+            // overriden
+            if (preg_match('#^/a/(?P<var>.*)$#s', $pathinfo, $matches)) {
+                $matches['_route'] = 'overriden';
+                return $matches;
+            }
+
+            if (0 === strpos($pathinfo, '/a/b\'b')) {
                 // foo2
                 if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#s', $pathinfo, $matches)) {
                     $matches['_route'] = 'foo2';
@@ -136,23 +145,27 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
                     $matches['_route'] = 'bar2';
                     return $matches;
                 }
+
             }
 
-            // overriden
+        }
-            if ($pathinfo === '/a/overriden2') {
+
-                return array('_route' => 'overriden');
+        if (0 === strpos($pathinfo, '/multi')) {
+            // helloWorld
+            if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?P<who>[^/]+?))?$#s', $pathinfo, $matches)) {
+                return array_merge($this->mergeDefaults($matches, array (  'who' => 'World!',)), array('_route' => 'helloWorld'));
             }
 
-            // ababa
+            // overriden2
-            if ($pathinfo === '/ababa') {
+            if ($pathinfo === '/multi/new') {
-                return array('_route' => 'ababa');
+                return array('_route' => 'overriden2');
             }
 
-            // foo4
+            // hey
-            if (preg_match('#^/aba/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
+            if ($pathinfo === '/multi/hey/') {
-                $matches['_route'] = 'foo4';
+                return array('_route' => 'hey');
-                return $matches;
             }
+
         }
 
         // foo3
@@ -167,6 +180,40 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
             return $matches;
         }
 
+        // ababa
+        if ($pathinfo === '/ababa') {
+            return array('_route' => 'ababa');
+        }
+
+        // foo4
+        if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
+            $matches['_route'] = 'foo4';
+            return $matches;
+        }
+
+        if (0 === strpos($pathinfo, '/a')) {
+            // a
+            if ($pathinfo === '/a/a...') {
+                return array('_route' => 'a');
+            }
+
+            if (0 === strpos($pathinfo, '/a/b')) {
+                // b
+                if (preg_match('#^/a/b/(?P<var>[^/]+?)$#s', $pathinfo, $matches)) {
+                    $matches['_route'] = 'b';
+                    return $matches;
+                }
+
+                // c
+                if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?P<var>[^/]+?)$#s', $pathinfo, $matches)) {
+                    $matches['_route'] = 'c';
+                    return $matches;
+                }
+
+            }
+
+        }
+
         throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
     }
 }

src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php

@@ -131,6 +131,15 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
                     return $matches;
                 }
 
+            }
+
+            // overriden
+            if (preg_match('#^/a/(?P<var>.*)$#s', $pathinfo, $matches)) {
+                $matches['_route'] = 'overriden';
+                return $matches;
+            }
+
+            if (0 === strpos($pathinfo, '/a/b\'b')) {
                 // foo2
                 if (preg_match('#^/a/b\'b/(?P<foo1>[^/]+?)$#s', $pathinfo, $matches)) {
                     $matches['_route'] = 'foo2';
@@ -142,23 +151,30 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
                     $matches['_route'] = 'bar2';
                     return $matches;
                 }
+
             }
 
-            // overriden
+        }
-            if ($pathinfo === '/a/overriden2') {
+
-                return array('_route' => 'overriden');
+        if (0 === strpos($pathinfo, '/multi')) {
+            // helloWorld
+            if (0 === strpos($pathinfo, '/multi/hello') && preg_match('#^/multi/hello(?:/(?P<who>[^/]+?))?$#s', $pathinfo, $matches)) {
+                return array_merge($this->mergeDefaults($matches, array (  'who' => 'World!',)), array('_route' => 'helloWorld'));
             }
 
-            // ababa
+            // overriden2
-            if ($pathinfo === '/ababa') {
+            if ($pathinfo === '/multi/new') {
-                return array('_route' => 'ababa');
+                return array('_route' => 'overriden2');
             }
 
-            // foo4
+            // hey
-            if (preg_match('#^/aba/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
+            if (rtrim($pathinfo, '/') === '/multi/hey') {
-                $matches['_route'] = 'foo4';
+                if (substr($pathinfo, -1) !== '/') {
-                return $matches;
+                    return $this->redirect($pathinfo.'/', 'hey');
+                }
+                return array('_route' => 'hey');
             }
+
         }
 
         // foo3
@@ -173,6 +189,40 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
             return $matches;
         }
 
+        // ababa
+        if ($pathinfo === '/ababa') {
+            return array('_route' => 'ababa');
+        }
+
+        // foo4
+        if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
+            $matches['_route'] = 'foo4';
+            return $matches;
+        }
+
+        if (0 === strpos($pathinfo, '/a')) {
+            // a
+            if ($pathinfo === '/a/a...') {
+                return array('_route' => 'a');
+            }
+
+            if (0 === strpos($pathinfo, '/a/b')) {
+                // b
+                if (preg_match('#^/a/b/(?P<var>[^/]+?)$#s', $pathinfo, $matches)) {
+                    $matches['_route'] = 'b';
+                    return $matches;
+                }
+
+                // c
+                if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?P<var>[^/]+?)$#s', $pathinfo, $matches)) {
+                    $matches['_route'] = 'c';
+                    return $matches;
+                }
+
+            }
+
+        }
+
         // secure
         if ($pathinfo === '/secure') {
             if ($this->context->getScheme() !== 'https') {

src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php

@@ -0,0 +1,44 @@
+<?php
+
+use Symfony\Component\Routing\Exception\MethodNotAllowedException;
+use Symfony\Component\Routing\Exception\ResourceNotFoundException;
+use Symfony\Component\Routing\RequestContext;
+
+/**
+ * ProjectUrlMatcher
+ *
+ * This class has been auto-generated
+ * by the Symfony Routing Component.
+ */
+class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
+{
+    /**
+     * Constructor.
+     */
+    public function __construct(RequestContext $context)
+    {
+        $this->context = $context;
+    }
+
+    public function match($pathinfo)
+    {
+        $allow = array();
+        $pathinfo = rawurldecode($pathinfo);
+
+        if (0 === strpos($pathinfo, '/rootprefix')) {
+            // static
+            if ($pathinfo === '/rootprefix/test') {
+                return array('_route' => 'static');
+            }
+
+            // dynamic
+            if (preg_match('#^/rootprefix/(?P<var>[^/]+?)$#s', $pathinfo, $matches)) {
+                $matches['_route'] = 'dynamic';
+                return $matches;
+            }
+
+        }
+
+        throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
+    }
+}

src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php

@@ -18,33 +18,6 @@ use Symfony\Component\Routing\RequestContext;
 
 class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
 {
-    public function testDump()
-    {
-        $collection = $this->getRouteCollection();
-
-        $dumper = new PhpMatcherDumper($collection, new RequestContext());
-
-        $this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher1.php', $dumper->dump(), '->dump() dumps basic routes to the correct PHP file.');
-
-        // force HTTPS redirection
-        $collection->add('secure', new Route(
-            '/secure',
-            array(),
-            array('_scheme' => 'https')
-        ));
-
-        // force HTTP redirection
-        $collection->add('nonsecure', new Route(
-            '/nonsecure',
-            array(),
-            array('_scheme' => 'http')
-        ));
-
-        $dumper = new PhpMatcherDumper($collection, new RequestContext());
-
-        $this->assertStringEqualsFile(__DIR__.'/../../Fixtures/dumper/url_matcher2.php', $dumper->dump(array('base_class' => 'Symfony\Component\Routing\Tests\Fixtures\RedirectableUrlMatcher')), '->dump() dumps basic routes to the correct PHP file.');
-    }
-
     /**
      * @expectedException \LogicException
      */
@@ -60,8 +33,22 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $dumper->dump();
     }
 
-    protected function getRouteCollection()
+    /**
+     * @dataProvider getRouteCollections
+     */
+    public function testDump(RouteCollection $collection, $fixture, $options = array())
     {
+        $basePath = __DIR__.'/../../Fixtures/dumper/';
+
+        $dumper = new PhpMatcherDumper($collection, new RequestContext());
+
+        $this->assertStringEqualsFile($basePath.$fixture, $dumper->dump($options), '->dump() correctly dumps routes as optimized PHP code.');
+    }
+
+    public function getRouteCollections()
+    {
+        /* test case 1 */
+
         $collection = new RouteCollection();
 
         $collection->add('overriden', new Route('/overriden'));
@@ -135,13 +122,25 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection1->add('bar1', new Route('/{bar}'));
         $collection2 = new RouteCollection();
         $collection2->addCollection($collection1, '/b\'b');
-        $collection2->add('overriden', new Route('/overriden2'));
+        $collection2->add('overriden', new Route('/{var}', array(), array('var' => '.*')));
         $collection1 = new RouteCollection();
         $collection1->add('foo2', new Route('/{foo1}'));
         $collection1->add('bar2', new Route('/{bar1}'));
         $collection2->addCollection($collection1, '/b\'b');
         $collection->addCollection($collection2, '/a');
 
+        // overriden through addCollection() and multiple sub-collections with no own prefix
+        $collection1 = new RouteCollection();
+        $collection1->add('overriden2', new Route('/old'));
+        $collection1->add('helloWorld', new Route('/hello/{who}', array('who' => 'World!')));
+        $collection2 = new RouteCollection();
+        $collection3 = new RouteCollection();
+        $collection3->add('overriden2', new Route('/new'));
+        $collection3->add('hey', new Route('/hey/'));
+        $collection1->addCollection($collection2);
+        $collection2->addCollection($collection3);
+        $collection->addCollection($collection1, '/multi');
+
         // "dynamic" prefix
         $collection1 = new RouteCollection();
         $collection1->add('foo3', new Route('/{foo}'));
@@ -150,13 +149,55 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection2->addCollection($collection1, '/b');
         $collection->addCollection($collection2, '/{_locale}');
 
+        // route between collections
         $collection->add('ababa', new Route('/ababa'));
 
-        // some more prefixes
+        // collection with static prefix but only one route
         $collection1 = new RouteCollection();
         $collection1->add('foo4', new Route('/{foo}'));
         $collection->addCollection($collection1, '/aba');
 
-        return $collection;
+        // multiple sub-collections with a single route and a prefix each
+        $collection1 = new RouteCollection();
+        $collection1->add('a', new Route('/a...'));
+        $collection2 = new RouteCollection();
+        $collection2->add('b', new Route('/{var}'));
+        $collection3 = new RouteCollection();
+        $collection3->add('c', new Route('/{var}'));
+
+        $collection1->addCollection($collection2, '/b');
+        $collection2->addCollection($collection3, '/c');
+        $collection->addCollection($collection1, '/a');
+
+        /* test case 2 */
+
+        $redirectCollection = clone $collection;
+
+        // force HTTPS redirection
+        $redirectCollection->add('secure', new Route(
+            '/secure',
+            array(),
+            array('_scheme' => 'https')
+        ));
+
+        // force HTTP redirection
+        $redirectCollection->add('nonsecure', new Route(
+            '/nonsecure',
+            array(),
+            array('_scheme' => 'http')
+        ));
+
+        /* test case 3 */
+
+        $rootprefixCollection = new RouteCollection();
+        $rootprefixCollection->add('static', new Route('/test'));
+        $rootprefixCollection->add('dynamic', new Route('/{var}'));
+        $rootprefixCollection->addPrefix('rootprefix');
+
+        return array(
+           array($collection, 'url_matcher1.php', array()),
+           array($redirectCollection, 'url_matcher2.php', array('base_class' => 'Symfony\Component\Routing\Tests\Fixtures\RedirectableUrlMatcher')),
+           array($rootprefixCollection, 'url_matcher3.php', array())
+        );
     }
 }

src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php

@@ -71,7 +71,7 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
 
     public function testMatch()
     {
-        // test the patterns are matched are parameters are returned
+        // test the patterns are matched and parameters are returned
         $collection = new RouteCollection();
         $collection->add('foo', new Route('/foo/{bar}'));
         $matcher = new UrlMatcher($collection, new RequestContext(), array());

src/Symfony/Component/Routing/Tests/RouteCollectionTest.php

@@ -28,7 +28,7 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @expectedException InvalidArgumentException
+     * @expectedException \InvalidArgumentException
      */
     public function testAddInvalidRoute()
     {
@@ -144,6 +144,8 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
             array('foo' => 'bar', 'compiler_class' => 'Symfony\\Component\\Routing\\RouteCompiler'),
             $collection->get('bar')->getOptions(), '->addPrefix() adds an option to all routes'
         );
+        $collection->addPrefix('0');
+        $this->assertEquals('/0/{admin}', $collection->getPrefix(), '->addPrefix() ensures a prefix must start with a slash and must not end with a slash');
     }
 
     public function testAddPrefixOverridesDefaultsAndRequirements()
@@ -180,4 +182,98 @@ class RouteCollectionTest extends \PHPUnit_Framework_TestCase
         $collection->addResource($foo = new FileResource(__DIR__.'/Fixtures/foo.xml'));
         $this->assertEquals(array($foo), $collection->getResources(), '->addResources() adds a resource');
     }
+
+    public function testUniqueRouteWithGivenName()
+    {
+        $collection1 = new RouteCollection();
+        $collection1->add('foo', new Route('/old'));
+        $collection2 = new RouteCollection();
+        $collection3 = new RouteCollection();
+        $collection3->add('foo', $new = new Route('/new'));
+
+        $collection1->addCollection($collection2);
+        $collection2->addCollection($collection3);
+
+        $collection1->add('stay', new Route('/stay'));
+
+        $iterator = $collection1->getIterator();
+
+        $this->assertSame($new, $collection1->get('foo'), '->get() returns new route that overrode previous one');
+        // size of 2 because collection1 contains collection2 and /stay but not /old anymore
+        $this->assertCount(2, $iterator, '->addCollection() removes previous routes when adding new routes with the same name');
+        $this->assertInstanceOf('Symfony\Component\Routing\RouteCollection', $iterator->current(), '->getIterator returns both Routes and RouteCollections');
+        $iterator->next();
+        $this->assertInstanceOf('Symfony\Component\Routing\Route', $iterator->current(), '->getIterator returns both Routes and RouteCollections');
+    }
+
+    public function testGet()
+    {
+        $collection1 = new RouteCollection();
+        $collection1->add('a', $a = new Route('/a'));
+        $collection2 = new RouteCollection();
+        $collection2->add('b', $b = new Route('/b'));
+        $collection1->addCollection($collection2);
+
+        $this->assertSame($b, $collection1->get('b'), '->get() returns correct route in child collection');
+        $this->assertNull($collection2->get('a'), '->get() does not return the route defined in parent collection');
+        $this->assertNull($collection1->get('non-existent'), '->get() returns null when route does not exist');
+        $this->assertNull($collection1->get(0), '->get() does not disclose internal child RouteCollection');
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testCannotSelfJoinCollection()
+    {
+        $collection = new RouteCollection();
+
+        $collection->addCollection($collection);
+    }
+
+    /**
+     * @expectedException \InvalidArgumentException
+     */
+    public function testCannotAddExistingCollectionToTree()
+    {
+        $collection1 = new RouteCollection();
+        $collection2 = new RouteCollection();
+        $collection3 = new RouteCollection();
+
+        $collection1->addCollection($collection2);
+        $collection1->addCollection($collection3);
+        $collection2->addCollection($collection3);
+    }
+
+    public function testPatternDoesNotChangeWhenDefinitionOrderChanges()
+    {
+        $collection1 = new RouteCollection();
+        $collection1->add('a', new Route('/a...'));
+        $collection2 = new RouteCollection();
+        $collection2->add('b', new Route('/b...'));
+        $collection3 = new RouteCollection();
+        $collection3->add('c', new Route('/c...'));
+
+        $rootCollection_A = new RouteCollection();
+        $collection2->addCollection($collection3, '/c');
+        $collection1->addCollection($collection2, '/b');
+        $rootCollection_A->addCollection($collection1, '/a');
+
+        // above should mean the same as below
+
+        $collection1 = new RouteCollection();
+        $collection1->add('a', new Route('/a...'));
+        $collection2 = new RouteCollection();
+        $collection2->add('b', new Route('/b...'));
+        $collection3 = new RouteCollection();
+        $collection3->add('c', new Route('/c...'));
+
+        $rootCollection_B = new RouteCollection();
+        $collection1->addCollection($collection2, '/b');
+        $collection2->addCollection($collection3, '/c');
+        $rootCollection_B->addCollection($collection1, '/a');
+
+        // test it now
+
+        $this->assertEquals($rootCollection_A, $rootCollection_B);
+    }
 }

src/Symfony/Component/Routing/Tests/RouteTest.php

@@ -98,10 +98,30 @@ class RouteTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals('\d+', $route->getRequirement('foo'), '->setRequirement() removes ^ and $ from the pattern');
     }
 
+    /**
+     * @dataProvider getInvalidRequirements
+     * @expectedException \InvalidArgumentException
+     */
+    public function testSetInvalidRequirement($req)
+    {
+        $route = new Route('/{foo}');
+        $route->setRequirement('foo', $req);
+    }
+
+    public function getInvalidRequirements()
+    {
+        return array(
+           array(''),
+           array(array())
+        );
+    }
+
     public function testCompile()
     {
         $route = new Route('/{foo}');
-        $this->assertEquals('Symfony\\Component\\Routing\\CompiledRoute', get_class($compiled = $route->compile()), '->compile() returns a compiled route');
+        $this->assertInstanceOf('Symfony\Component\Routing\CompiledRoute', $compiled = $route->compile(), '->compile() returns a compiled route');
-        $this->assertEquals($compiled, $route->compile(), '->compile() only compiled the route once');
+        $this->assertSame($compiled, $route->compile(), '->compile() only compiled the route once if unchanged');
+        $route->setRequirement('foo', '.*');
+        $this->assertNotSame($compiled, $route->compile(), '->compile() recompiles if the route was modified');
     }
 }