[Routing] removed tree structure from RouteCollection

src/Symfony/Bundle/FrameworkBundle/Routing/Router.php

@@ -84,19 +84,15 @@ class Router extends BaseRouter implements WarmableInterface
     private function resolveParameters(RouteCollection $collection)
     {
         foreach ($collection as $route) {
-            if ($route instanceof RouteCollection) {
+            foreach ($route->getDefaults() as $name => $value) {
-                $this->resolveParameters($route);
+                $route->setDefault($name, $this->resolve($value));
-            } else {
-                foreach ($route->getDefaults() as $name => $value) {
-                    $route->setDefault($name, $this->resolve($value));
-                }
-
-                foreach ($route->getRequirements() as $name => $value) {
-                     $route->setRequirement($name, $this->resolve($value));
-                }
-
-                $route->setPattern($this->resolve($route->getPattern()));
             }
+
+            foreach ($route->getRequirements() as $name => $value) {
+                 $route->setRequirement($name, $this->resolve($value));
+            }
+
+            $route->setPattern($this->resolve($route->getPattern()));
         }
     }
 

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

@@ -111,7 +111,6 @@ EOF;
     {
         $fetchedHostname = false;
 
-        $routes = $this->flattenRouteCollection($routes);
         $groups = $this->groupRoutesByHostnameRegex($routes);
         $code = '';
 
@@ -321,31 +320,6 @@ EOF;
         return $code;
     }
 
-    /**
-     * Flattens a tree of routes to a single collection.
-     *
-     * @param RouteCollection       $routes Collection of routes
-     * @param DumperCollection|null $to     A DumperCollection to add routes to
-     *
-     * @return DumperCollection
-     */
-    private function flattenRouteCollection(RouteCollection $routes, DumperCollection $to = null)
-    {
-        if (null === $to) {
-            $to = new DumperCollection();
-        }
-
-        foreach ($routes as $name => $route) {
-            if ($route instanceof RouteCollection) {
-                $this->flattenRouteCollection($route, $to);
-            } else {
-                $to->add(new DumperRoute($name, $route));
-            }
-        }
-
-        return $to;
-    }
-
     /**
      * Groups consecutive routes having the same hostname regex.
      *
@@ -355,7 +329,7 @@ EOF;
      *
      * @return DumperCollection A collection with routes grouped by hostname regex in sub-collections
      */
-    private function groupRoutesByHostnameRegex(DumperCollection $routes)
+    private function groupRoutesByHostnameRegex(RouteCollection $routes)
     {
         $groups = new DumperCollection();
 
@@ -363,14 +337,14 @@ EOF;
         $currentGroup->setAttribute('hostname_regex', null);
         $groups->add($currentGroup);
 
-        foreach ($routes as $route) {
+        foreach ($routes as $name => $route) {
-            $hostnameRegex = $route->getRoute()->compile()->getHostnameRegex();
+            $hostnameRegex = $route->compile()->getHostnameRegex();
             if ($currentGroup->getAttribute('hostname_regex') !== $hostnameRegex) {
                 $currentGroup = new DumperCollection();
                 $currentGroup->setAttribute('hostname_regex', $hostnameRegex);
                 $groups->add($currentGroup);
             }
-            $currentGroup->add($route);
+            $currentGroup->add(new DumperRoute($name, $route));
         }
 
         return $groups;

src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php

@@ -44,14 +44,6 @@ class TraceableUrlMatcher extends UrlMatcher
     protected function matchCollection($pathinfo, RouteCollection $routes)
     {
         foreach ($routes as $name => $route) {
-            if ($route instanceof RouteCollection) {
-                if (!$ret = $this->matchCollection($pathinfo, $route)) {
-                    continue;
-                }
-
-                return true;
-            }
-
             $compiledRoute = $route->compile();
 
             if (!preg_match($compiledRoute->getRegex(), $pathinfo, $matches)) {

src/Symfony/Component/Routing/Matcher/UrlMatcher.php

@@ -105,18 +105,6 @@ class UrlMatcher implements UrlMatcherInterface
     protected function matchCollection($pathinfo, RouteCollection $routes)
     {
         foreach ($routes as $name => $route) {
-            if ($route instanceof RouteCollection) {
-                if (false === strpos($route->getPrefix(), '{') && $route->getPrefix() !== substr($pathinfo, 0, strlen($route->getPrefix()))) {
-                    continue;
-                }
-
-                if (!$ret = $this->matchCollection($pathinfo, $route)) {
-                    continue;
-                }
-
-                return $ret;
-            }
-
             $compiledRoute = $route->compile();
 
             // check the static prefix of the URL first. Only use the more expensive preg_match when it matches

src/Symfony/Component/Routing/RouteCollection.php

@@ -14,10 +14,11 @@ namespace Symfony\Component\Routing;
 use Symfony\Component\Config\Resource\ResourceInterface;
 
 /**
- * A RouteCollection represents a set of Route instances as a tree structure.
+ * A RouteCollection represents a set of Route instances.
  *
- * When adding a route, it overrides existing routes with the
+ * When adding a route at the end of the collection, an existing route
- * same name defined in the instance or its children and parents.
+ * with the same name is removed first. So there can only be one route
+ * with a given name.
  *
  * @author Fabien Potencier <fabien@symfony.com>
  * @author Tobias Schultze <http://tobion.de>
@@ -27,7 +28,7 @@ use Symfony\Component\Config\Resource\ResourceInterface;
 class RouteCollection implements \IteratorAggregate, \Countable
 {
     /**
-     * @var (RouteCollection|Route)[]
+     * @var Route[]
      */
     private $routes = array();
 
@@ -43,6 +44,7 @@ class RouteCollection implements \IteratorAggregate, \Countable
 
     /**
      * @var RouteCollection|null
+     * @deprecated since version 2.2, will be removed in 2.3
      */
     private $parent;
 
@@ -50,9 +52,6 @@ class RouteCollection implements \IteratorAggregate, \Countable
     {
         foreach ($this->routes as $name => $route) {
             $this->routes[$name] = clone $route;
-            if ($route instanceof RouteCollection) {
-                $this->routes[$name]->setParent($this);
-            }
         }
     }
 
@@ -60,6 +59,8 @@ class RouteCollection implements \IteratorAggregate, \Countable
      * Gets the parent RouteCollection.
      *
      * @return RouteCollection|null The parent RouteCollection or null when it's the root
+     *
+     * @deprecated since version 2.2, will be removed in 2.3
      */
     public function getParent()
     {
@@ -67,9 +68,11 @@ class RouteCollection implements \IteratorAggregate, \Countable
     }
 
     /**
-     * Gets the root RouteCollection of the tree.
+     * Gets the root RouteCollection.
      *
      * @return RouteCollection The root RouteCollection
+     *
+     * @deprecated since version 2.2, will be removed in 2.3
      */
     public function getRoot()
     {
@@ -82,9 +85,13 @@ class RouteCollection implements \IteratorAggregate, \Countable
     }
 
     /**
-     * Gets the current RouteCollection as an Iterator that includes all routes and child route collections.
+     * Gets the current RouteCollection as an Iterator that includes all routes.
+     * 
+     * It implements \IteratorAggregate.
      *
-     * @return \ArrayIterator An \ArrayIterator interface
+     * @see all()
+     *
+     * @return \ArrayIterator An \ArrayIterator object for iterating over routes
      */
     public function getIterator()
     {
@@ -94,16 +101,11 @@ class RouteCollection implements \IteratorAggregate, \Countable
     /**
      * Gets the number of Routes in this collection.
      *
-     * @return int The number of routes in this collection, including nested collections
+     * @return int The number of routes
      */
     public function count()
     {
-        $count = 0;
+        return count($this->routes);
-        foreach ($this->routes as $route) {
-            $count += $route instanceof RouteCollection ? count($route) : 1;
-        }
-
-        return $count;
     }
 
     /**
@@ -122,32 +124,23 @@ class RouteCollection implements \IteratorAggregate, \Countable
             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));
         }
 
-        $this->remove($name);
+        unset($this->routes[$name]);
 
         $this->routes[$name] = $route;
     }
 
     /**
-     * Returns all routes in this collection and its children.
+     * Returns all routes in this collection.
      *
      * @return Route[] An array of routes
      */
     public function all()
     {
-        $routes = array();
+        return $this->routes;
-        foreach ($this->routes as $name => $route) {
-            if ($route instanceof RouteCollection) {
-                $routes = array_merge($routes, $route->all());
-            } else {
-                $routes[$name] = $route;
-            }
-        }
-
-        return $routes;
     }
 
     /**
-     * Gets a route by name defined in this collection or its children.
+     * Gets a route by name.
      *
      * @param string $name The route name
      *
@@ -155,36 +148,31 @@ class RouteCollection implements \IteratorAggregate, \Countable
      */
     public function get($name)
     {
-        if (isset($this->routes[$name])) {
+        return isset($this->routes[$name]) ? $this->routes[$name] : null;
-            return $this->routes[$name] instanceof RouteCollection ? null : $this->routes[$name];
-        }
-
-        foreach ($this->routes as $routes) {
-            if ($routes instanceof RouteCollection && null !== $route = $routes->get($name)) {
-                return $route;
-            }
-        }
-
-        return null;
     }
 
     /**
-     * Removes a route or an array of routes by name from all connected
+     * Removes a route or an array of routes by name from the collection
-     * collections (this instance and all parents and children).
+     *
+     * For BC it's also removed from the root, which will not be the case in 2.3
+     * as the RouteCollection won't be a tree structure.
      *
      * @param string|array $name The route name or an array of route names
      */
     public function remove($name)
     {
+        // just for BC
         $root = $this->getRoot();
 
         foreach ((array) $name as $n) {
-            $root->removeRecursively($n);
+            unset($root->routes[$n]);
+            unset($this->routes[$n]);
         }
     }
 
     /**
-     * Adds a route collection to the current set of routes (at the end of the current set).
+     * Adds a route collection at the end of the current set by appending all
+     * routes of the added collection.
      *
      * @param RouteCollection $collection      A RouteCollection instance
      * @param string          $prefix          An optional prefix to add before each pattern of the route collection
@@ -193,22 +181,16 @@ class RouteCollection implements \IteratorAggregate, \Countable
      * @param array           $options         An array of options
      * @param string          $hostnamePattern Hostname pattern
      *
-     * @throws \InvalidArgumentException When the RouteCollection already exists in the tree
-     *
      * @api
      */
     public function addCollection(RouteCollection $collection, $prefix = '', $defaults = array(), $requirements = array(), $options = array(), $hostnamePattern = '')
     {
-        // prevent infinite loops by recursive referencing
+        // This is to keep BC for getParent() and getRoot(). It does not prevent
-        $root = $this->getRoot();
+        // infinite loops by recursive referencing. But we don't need that logic
-        if ($root === $collection || $root->hasCollection($collection)) {
+        // anymore as the tree logic has been deprecated and we are just widening
-            throw new \InvalidArgumentException('The RouteCollection already exists in the tree.');
+        // the accepted range.
-        }
+        $collection->parent = $this;
 
-        // 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);
@@ -217,7 +199,14 @@ class RouteCollection implements \IteratorAggregate, \Countable
             $collection->setHostnamePattern($hostnamePattern);
         }
 
-        $this->routes[] = $collection;
+        // we need to remove all routes with the same names first because just replacing them
+        // would not place the new route at the end of the merged array
+        foreach ($collection->all() as $name => $route) {
+            unset($this->routes[$name]);
+            $this->routes[$name] = $route;
+        }
+
+        $this->resources = array_merge($this->resources, $collection->getResources());
     }
 
     /**
@@ -244,17 +233,12 @@ class RouteCollection implements \IteratorAggregate, \Countable
         }
 
         foreach ($this->routes as $route) {
-            if ($route instanceof RouteCollection) {
+            if ('' !== $prefix) {
-                // we add the slashes so the prefix is not lost by trimming in the sub-collection
+                $route->setPattern('/' . $prefix . $route->getPattern());
-                $route->addPrefix('/' . $prefix . '/', $defaults, $requirements, $options);
-            } else {
-                if ('' !== $prefix) {
-                    $route->setPattern('/' . $prefix . $route->getPattern());
-                }
-                $route->addDefaults($defaults);
-                $route->addRequirements($requirements);
-                $route->addOptions($options);
             }
+            $route->addDefaults($defaults);
+            $route->addRequirements($requirements);
+            $route->addOptions($options);
         }
     }
 
@@ -269,7 +253,7 @@ class RouteCollection implements \IteratorAggregate, \Countable
     }
 
     /**
-     * Sets the hostname pattern on all child routes.
+     * Sets the hostname pattern on all routes.
      *
      * @param string $pattern The pattern
      */
@@ -287,14 +271,7 @@ class RouteCollection implements \IteratorAggregate, \Countable
      */
     public function getResources()
     {
-        $resources = $this->resources;
+        return array_unique($this->resources);
-        foreach ($this->routes as $routes) {
-            if ($routes instanceof RouteCollection) {
-                $resources = array_merge($resources, $routes->getResources());
-            }
-        }
-
-        return array_unique($resources);
     }
 
     /**
@@ -306,60 +283,4 @@ class RouteCollection implements \IteratorAggregate, \Countable
     {
         $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
-        // iterating 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;
-    }
 }