diff --git a/CHANGELOG-2.1.md b/CHANGELOG-2.1.md index e87a58f4a4d63..f217f7c1627ee 100644 --- a/CHANGELOG-2.1.md +++ b/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 - decoded twice before. Note that the `urldecode()` calls have been change for a - single `rawurldecode()` in order to support `+` for input paths. + * [BC BREAK] the UrlMatcher urldecodes the route parameters only once, they were decoded twice before. + Note that the `urldecode()` calls have been changed for a 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 diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php index dd7fbe35442af..447a51fd7f688 100644 --- a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php +++ b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php @@ -18,6 +18,7 @@ * PhpMatcherDumper creates a PHP class able to match URLs for a given set of routes. * * @author Fabien Potencier + * @author Tobias Schultze */ class PhpMatcherDumper extends MatcherDumper { @@ -44,21 +45,47 @@ public function dump(array $options = array()) $interfaces = class_implements($options['base_class']); $supportsRedirections = isset($interfaces['Symfony\Component\Routing\Matcher\RedirectableUrlMatcherInterface']); - return - $this->startClass($options['class'], $options['base_class']). - $this->addConstructor(). - $this->addMatcher($supportsRedirections). - $this->endClass() - ; + return <<context = \$context; + } + +{$this->generateMatchMethod($supportsRedirections)} +} + +EOF; } - private function addMatcher($supportsRedirections) + /** + * 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) { - // 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"); + $code = rtrim($this->compileRoutes($this->getRoutes(), $supportsRedirections), "\n"); return <<getIterator(); - $keys = array_keys($routeIterator->getArrayCopy()); - $keysCount = count($keys); - - $i = 0; - foreach ($routeIterator as $name => $route) { - $i++; - - if ($route instanceof RouteCollection) { - $prefix = $route->getPrefix(); - $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"; + $prefix = $routes->getPrefix(); + $countDirectChildRoutes = $this->countDirectChildRoutes($routes); + $countAllChildRoutes = count($routes->all()); + $optimizable = '' !== $prefix && $prefix !== $parentPrefix && $countDirectChildRoutes > 0 && $countAllChildRoutes > 1 && false === strpos($prefix, '{'); + // whether the matching of routes within the collection can 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 + if ($optimizable) { + $code .= sprintf(" if (0 === strpos(\$pathinfo, %s)) {\n", var_export($prefix, true)); + } + + foreach ($routes as $name => $route) { + if ($route instanceof Route) { + $code .= $this->compileRoute($route, $name, $supportsRedirections, 1 === $countAllChildRoutes ? null : $prefix)."\n"; + // a single route in a sub-collection is not wrapped so it should do its own optimization in ->compileRoute with $parentPrefix = null + } elseif ($countAllChildRoutes - $countDirectChildRoutes > 0) { // we can stop iterating recursively if we already know there are no more routes + $code .= $this->compileRoutes($route, $supportsRedirections, $prefix); } } + if ($optimizable) { + $code .= " }\n\n"; + $code = preg_replace('/^.{2,}$/m', ' $0', $code); // apply extra indention at each line (except empty ones) + } + 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 +198,7 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren $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 +285,4 @@ private function compileRoute(Route $route, $name, $supportsRedirections, $paren return $code; } - - private function startClass($class, $baseClass) - { - return <<context = \$context; - } - -EOF; - } - - private function endClass() - { - return <<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 @@ public function addOptions(array $options) foreach ($options as $name => $option) { $this->options[(string) $name] = $option; } + $this->compiled = null; return $this; } @@ -147,6 +150,7 @@ public function addOptions(array $options) public function setOption($name, $value) { $this->options[$name] = $value; + $this->compiled = null; return $this; } @@ -203,6 +207,7 @@ public function addDefaults(array $defaults) foreach ($defaults as $name => $default) { $this->defaults[(string) $name] = $default; } + $this->compiled = null; return $this; } @@ -244,6 +249,7 @@ public function hasDefault($name) public function setDefault($name, $default) { $this->defaults[(string) $name] = $default; + $this->compiled = null; return $this; } @@ -288,6 +294,7 @@ public function addRequirements(array $requirements) foreach ($requirements as $key => $regex) { $this->requirements[$key] = $this->sanitizeRequirement($key, $regex); } + $this->compiled = null; return $this; } @@ -317,6 +324,7 @@ public function getRequirement($key) public function setRequirement($key, $regex) { $this->requirements[$key] = $this->sanitizeRequirement($key, $regex); + $this->compiled = null; return $this; } @@ -343,15 +351,19 @@ public function compile() private function sanitizeRequirement($key, $regex) { - if (is_array($regex)) { - throw new \InvalidArgumentException(sprintf('Routing requirements must be a string, array given for "%s"', $key)); + if (!is_string($regex)) { + throw new \InvalidArgumentException(sprintf('Routing requirement for "%s" must be a string', $key)); + } + + if ('' === $regex) { + throw new \InvalidArgumentException(sprintf('Routing requirement for "%s" cannot be empty', $key)); } - if ('^' == $regex[0]) { + if ('^' === $regex[0]) { $regex = substr($regex, 1); } - if ('$' == substr($regex, -1)) { + if ('$' === substr($regex, -1)) { $regex = substr($regex, 0, -1); } diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index f049d5985b1c8..59a00b0e9b056 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/src/Symfony/Component/Routing/RouteCollection.php @@ -14,12 +14,13 @@ 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 + * @author Tobias Schultze * * @api */ @@ -55,7 +56,7 @@ public function __clone() /** * 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 @@ public function getParent() } /** - * 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 @@ public function add($name, Route $route) 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; - while ($parent->getParent()) { - $parent = $parent->getParent(); - } - - if ($parent) { - $parent->remove($name); - } + $this->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 @@ public function all() } /** - * 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 - foreach (array_reverse($this->routes) as $routes) { - if (!$routes instanceof RouteCollection) { - continue; - } + if (isset($this->routes[$name])) { + return $this->routes[$name] instanceof RouteCollection ? null : $this->routes[$name]; + } - 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 $this->routes[$name]; - } + return null; } /** - * 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])) { - unset($this->routes[$name]); - } + $root = $this->getRoot(); - foreach ($this->routes as $routes) { - if ($routes instanceof RouteCollection) { - $routes->remove($name); - } + foreach ((array) $name as $n) { + $root->removeRecursively($n); } } @@ -181,18 +174,25 @@ public function remove($name) * @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); - $collection->addPrefix($prefix, $defaults, $requirements, $options); - - // remove all routes with the same name in all existing collections - foreach (array_keys($collection->all()) as $name) { - $this->remove($name); + // 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.'); } + // 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,8 +211,12 @@ public function addPrefix($prefix, $defaults = array(), $requirements = array(), // 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; } @@ -230,6 +234,12 @@ public function addPrefix($prefix, $defaults = array(), $requirements = array(), } } + /** + * Returns the prefix that may contain placeholders. + * When given, it must start with a slash and must not end with a slash. + * + * @return string The prefix + */ public function getPrefix() { return $this->prefix; @@ -261,4 +271,60 @@ public function addResource(ResourceInterface $resource) { $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; + } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php index 98d8b0ef90b16..4753410ffa557 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php @@ -125,6 +125,15 @@ public function match($pathinfo) return $matches; } + } + + // overriden + if (preg_match('#^/a/(?P.*)$#s', $pathinfo, $matches)) { + $matches['_route'] = 'overriden'; + return $matches; + } + + if (0 === strpos($pathinfo, '/a/b\'b')) { // foo2 if (preg_match('#^/a/b\'b/(?P[^/]+?)$#s', $pathinfo, $matches)) { $matches['_route'] = 'foo2'; @@ -136,23 +145,27 @@ public function match($pathinfo) $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[^/]+?))?$#s', $pathinfo, $matches)) { + return array_merge($this->mergeDefaults($matches, array ( 'who' => 'World!',)), array('_route' => 'helloWorld')); } - // ababa - if ($pathinfo === '/ababa') { - return array('_route' => 'ababa'); + // overriden2 + if ($pathinfo === '/multi/new') { + return array('_route' => 'overriden2'); } - // foo4 - if (preg_match('#^/aba/(?P[^/]+?)$#s', $pathinfo, $matches)) { - $matches['_route'] = 'foo4'; - return $matches; + // hey + if ($pathinfo === '/multi/hey/') { + return array('_route' => 'hey'); } + } // foo3 @@ -167,6 +180,40 @@ public function match($pathinfo) return $matches; } + // ababa + if ($pathinfo === '/ababa') { + return array('_route' => 'ababa'); + } + + // foo4 + if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?P[^/]+?)$#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[^/]+?)$#s', $pathinfo, $matches)) { + $matches['_route'] = 'b'; + return $matches; + } + + // c + if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?P[^/]+?)$#s', $pathinfo, $matches)) { + $matches['_route'] = 'c'; + return $matches; + } + + } + + } + throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException(); } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php index 390acfbd245b4..dbfd28ab0a9ba 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php @@ -131,6 +131,15 @@ public function match($pathinfo) return $matches; } + } + + // overriden + if (preg_match('#^/a/(?P.*)$#s', $pathinfo, $matches)) { + $matches['_route'] = 'overriden'; + return $matches; + } + + if (0 === strpos($pathinfo, '/a/b\'b')) { // foo2 if (preg_match('#^/a/b\'b/(?P[^/]+?)$#s', $pathinfo, $matches)) { $matches['_route'] = 'foo2'; @@ -142,23 +151,30 @@ public function match($pathinfo) $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[^/]+?))?$#s', $pathinfo, $matches)) { + return array_merge($this->mergeDefaults($matches, array ( 'who' => 'World!',)), array('_route' => 'helloWorld')); } - // ababa - if ($pathinfo === '/ababa') { - return array('_route' => 'ababa'); + // overriden2 + if ($pathinfo === '/multi/new') { + return array('_route' => 'overriden2'); } - // foo4 - if (preg_match('#^/aba/(?P[^/]+?)$#s', $pathinfo, $matches)) { - $matches['_route'] = 'foo4'; - return $matches; + // hey + if (rtrim($pathinfo, '/') === '/multi/hey') { + if (substr($pathinfo, -1) !== '/') { + return $this->redirect($pathinfo.'/', 'hey'); + } + return array('_route' => 'hey'); } + } // foo3 @@ -173,6 +189,40 @@ public function match($pathinfo) return $matches; } + // ababa + if ($pathinfo === '/ababa') { + return array('_route' => 'ababa'); + } + + // foo4 + if (0 === strpos($pathinfo, '/aba') && preg_match('#^/aba/(?P[^/]+?)$#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[^/]+?)$#s', $pathinfo, $matches)) { + $matches['_route'] = 'b'; + return $matches; + } + + // c + if (0 === strpos($pathinfo, '/a/b/c') && preg_match('#^/a/b/c/(?P[^/]+?)$#s', $pathinfo, $matches)) { + $matches['_route'] = 'c'; + return $matches; + } + + } + + } + // secure if ($pathinfo === '/secure') { if ($this->context->getScheme() !== 'https') { diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php new file mode 100644 index 0000000000000..2dc0be9886012 --- /dev/null +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php @@ -0,0 +1,44 @@ +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[^/]+?)$#s', $pathinfo, $matches)) { + $matches['_route'] = 'dynamic'; + return $matches; + } + + } + + throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException(); + } +} diff --git a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php index 293348cc15201..b4dde1d4803ec 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php @@ -18,33 +18,6 @@ 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 @@ public function testDumpWhenSchemeIsUsedWithoutAProperDumper() $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 @@ protected function getRouteCollection() $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,56 @@ protected function getRouteCollection() $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()) + ); } } diff --git a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php index 1d782b889d1c3..e38c75a0303f2 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php @@ -71,7 +71,7 @@ public function testMethodNotAllowedAggregatesAllowedMethods() 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()); diff --git a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php index 3e73065d0b7ca..f5e84124ed36e 100644 --- a/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteCollectionTest.php @@ -28,7 +28,7 @@ public function testRoute() } /** - * @expectedException InvalidArgumentException + * @expectedException \InvalidArgumentException */ public function testAddInvalidRoute() { @@ -144,6 +144,8 @@ public function testAddPrefix() 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 @@ public function testResource() $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 testDefinitionOrderDoesNotMatter() + { + $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); + } } diff --git a/src/Symfony/Component/Routing/Tests/RouteTest.php b/src/Symfony/Component/Routing/Tests/RouteTest.php index 1295bc43c0b8c..8e466826dae35 100644 --- a/src/Symfony/Component/Routing/Tests/RouteTest.php +++ b/src/Symfony/Component/Routing/Tests/RouteTest.php @@ -98,10 +98,30 @@ public function testRequirement() $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->assertEquals($compiled, $route->compile(), '->compile() only compiled the route once'); + $this->assertInstanceOf('Symfony\Component\Routing\CompiledRoute', $compiled = $route->compile(), '->compile() returns a compiled route'); + $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'); } }