diogo
/
symfony
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

merged branch vicb/routing_improvements (PR #3876) 

Commits
-------

9307f5b [Routing] Implement bug fixes and enhancements

Discussion
----------

[Routing] Implement bug fixes and enhancements (from @Tobion)

This is mainly #3754 with some minor formatting changes.

Original PR message from @Tobion:

Here a list of what is fixed. Tests pass.

1. The `RouteCollection` states

    > it overrides existing routes with the same name defined in the instance or its children and parents.

    But this is not true for `->addCollection()` but only for `->add()`. addCollection does not remove routes with the same name in its parents (only in its children). I don't think this is on purpose.
    So I fixed it by making sure there can only be one route per name in all connected collections. This way we can also simplify `->get()` and `->remove()` and improve performance since we can stop iterating recursively when we found the first and only route with a given name.
    See `testUniqueRouteWithGivenName` that fails in the old code but works now.

2. There was an bug with `$collection->addPrefix('0');` that didn't apply the starting slash. Fixed and test case added.

3. There is an issue with `->get()` that I also think is not intended. Currently it allows to access a sub-RouteCollection by specifing $name as array integer index. But according to the PHPdoc you should only be allowed to receive a Route instance and not a RouteCollection.
    See `testGet` that has a failing test case. I fixed this behavior.

4. Then I recognized that `->addCollection` depended on the order of applying them. So

        $collection1->addCollection($collection2, '/b');
        $collection2->addCollection($collection3, '/c');
        $rootCollection->addCollection($collection1, '/a');

    had a different pattern result from

        $collection2->addCollection($collection3, '/c');
        $collection1->addCollection($collection2, '/b');
        $rootCollection->addCollection($collection1, '/a');

    Fixed and test case added. See `testPatternDoesNotChangeWhenDefinitionOrderChanges`.

5. PHP could have ended in an infinite loop when one tried to add an existing RouteCollection to the tree. Fixed by throwing an exception when this situation is detected. See tests `testCannotSelfJoinCollection` and `testCannotAddExistingCollectionToTree`.

6. I made `setParent()` private because its not useful outside the class itself. And `remove()` also removes the route from its parents. Added public `getRoot()` method.

7. The `Route` class throwed a PHP warning when trying to set an empty requirement.

8. Fixed issue #3777. See discussion there for more info. I fixed it by removing the over-optimization that was introduced in 91f4097a09 but didn't work properly. One cannot reorder the route definitions, as is was done, because then the wrong route might me matched before the correct one. If one really wanted to do that, it would require to calculate the intersection of two regular expressions to determine if they can be grouped together (a tool that would also be useful to check whether a route is unreachable, see discussion in #3678) We can only safely optimize routes with a static prefix within a RouteCollection, not across multiple RouteCollections with different parents.  This is especially true when using variables and regular expressions requirements.
I could however apply an optimization that was missing yet: Collections with a single route were missing the static prefix optimization with `0 === strpos()`.

9. Fixed an issue where the `PhpMatcherDumper` would not apply the optimization if the root collection to be dumped has a prefix itself. For this I had to rewrite `compileRoutes`. It is also much easier to understand now. Addionally I added many comments and PHPdoc because complex recursive methods like this are still hard to grasp.
I added a test case for this (`url_matcher3.php`).

10. Fix that `Route::compile` needs to recompile a route once it is modified. Otherwise we have a wrong result. Test case added.
This commit is contained in:
Fabien Potencier 2012-04-11 15:53:10 +02:00
parent 3469713d90 9307f5b33b
commit 0c57db1771
11 changed files with 590 additions and 224 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
CHANGELOG-2.1.md
View File

@ -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

221
src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php
View File

@ -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']);
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");
$supportsRedirections = isset($interfaces['Symfony\\Component\\Routing\\Matcher\\RedirectableUrlMatcherInterface']);
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();
$keys = array_keys($routeIterator->getArrayCopy());
$keysCount = count($keys);
$prefix = $routes->getPrefix();
$countDirectChildRoutes = $this->countDirectChildRoutes($routes);
$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 ($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";
foreach ($routes as $name => $route) {
if ($route instanceof Route) {
// 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";
} 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";
// 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;
}
}

22
src/Symfony/Component/Routing/Route.php
View File

@ -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)) {
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[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);
}

155
src/Symfony/Component/Routing/RouteCollection.php
View File

@ -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;
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 @@ 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
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 @@ 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);
$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,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;
}
}

67
src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php
View File

@ -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
if ($pathinfo === '/ababa') {
return array('_route' => 'ababa');
// overriden2
if ($pathinfo === '/multi/new') {
return array('_route' => 'overriden2');
}
// foo4
if (preg_match('#^/aba/(?P<foo>[^/]+?)$#s', $pathinfo, $matches)) {
$matches['_route'] = 'foo4';
return $matches;
// hey
if ($pathinfo === '/multi/hey/') {
return array('_route' => 'hey');
}
}
// 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();
}
}

70
src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php
View File

@ -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
if ($pathinfo === '/ababa') {
return array('_route' => 'ababa');
// overriden2
if ($pathinfo === '/multi/new') {
return array('_route' => 'overriden2');
}
// foo4
if (preg_match('#^/aba/(?P<foo>[^/]+?)$#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 @@ 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') {

44
src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher3.php Normal file
View File

@ -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();
}
}

103
src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php
View File

@ -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())
);
}
}

2
src/Symfony/Component/Routing/Tests/Matcher/UrlMatcherTest.php
View File

@ -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());

98
src/Symfony/Component/Routing/Tests/RouteCollectionTest.php
View File

@ -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);
}
}

24
src/Symfony/Component/Routing/Tests/RouteTest.php
View File

@ -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->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');
}
}