From 98f3ca83959e71f91e54315d7f50923a86ba3a45 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Mon, 26 Nov 2012 18:28:37 +0100 Subject: [PATCH] [Routing] removed tree structure from RouteCollection --- .../Bundle/FrameworkBundle/Routing/Router.php | 20 +- .../Matcher/Dumper/PhpMatcherDumper.php | 34 +--- .../Routing/Matcher/TraceableUrlMatcher.php | 8 - .../Component/Routing/Matcher/UrlMatcher.php | 12 -- .../Component/Routing/RouteCollection.php | 185 +++++------------- 5 files changed, 65 insertions(+), 194 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php b/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php index 31fbc63ef6..cc6df98a41 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Routing/Router.php +++ b/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) { - $this->resolveParameters($route); - } 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->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())); } } diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php index 454e26ca7b..2c86761b63 100644 --- a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php +++ b/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) { - $hostnameRegex = $route->getRoute()->compile()->getHostnameRegex(); + foreach ($routes as $name => $route) { + $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; diff --git a/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php b/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php index 1e2491c488..6ef846dd69 100644 --- a/src/Symfony/Component/Routing/Matcher/TraceableUrlMatcher.php +++ b/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)) { diff --git a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php b/src/Symfony/Component/Routing/Matcher/UrlMatcher.php index 5c6985447c..7a12dd16b9 100644 --- a/src/Symfony/Component/Routing/Matcher/UrlMatcher.php +++ b/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 diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index ebbf236b58..91efdf4a71 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/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 - * same name defined in the instance or its children and parents. + * When adding a route at the end of the collection, an existing route + * with the same name is removed first. So there can only be one route + * with a given name. * * @author Fabien Potencier * @author Tobias Schultze @@ -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; - foreach ($this->routes as $route) { - $count += $route instanceof RouteCollection ? count($route) : 1; - } - - return $count; + return count($this->routes); } /** @@ -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(); - foreach ($this->routes as $name => $route) { - if ($route instanceof RouteCollection) { - $routes = array_merge($routes, $route->all()); - } else { - $routes[$name] = $route; - } - } - - return $routes; + return $this->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 $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; + return isset($this->routes[$name]) ? $this->routes[$name] : null; } /** - * Removes a route or an array of routes by name from all connected - * collections (this instance and all parents and children). + * Removes a route or an array of routes by name from the collection + * + * 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 - $root = $this->getRoot(); - if ($root === $collection || $root->hasCollection($collection)) { - throw new \InvalidArgumentException('The RouteCollection already exists in the tree.'); - } + // This is to keep BC for getParent() and getRoot(). It does not prevent + // infinite loops by recursive referencing. But we don't need that logic + // anymore as the tree logic has been deprecated and we are just widening + // 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) { - // we add the slashes so the prefix is not lost by trimming in the sub-collection - $route->addPrefix('/' . $prefix . '/', $defaults, $requirements, $options); - } else { - if ('' !== $prefix) { - $route->setPattern('/' . $prefix . $route->getPattern()); - } - $route->addDefaults($defaults); - $route->addRequirements($requirements); - $route->addOptions($options); + if ('' !== $prefix) { + $route->setPattern('/' . $prefix . $route->getPattern()); } + $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; - foreach ($this->routes as $routes) { - if ($routes instanceof RouteCollection) { - $resources = array_merge($resources, $routes->getResources()); - } - } - - return array_unique($resources); + return array_unique($this->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; - } }