diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php index 5c2fcb4b75..9df0176407 100644 --- a/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php +++ b/src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php @@ -55,7 +55,7 @@ class PhpMatcherDumper extends MatcherDumper private function addMatcher($supportsRedirections) { // we need to deep clone the routes as we will modify the structure to optimize the dump - $code = implode("\n", $this->compileRoutes(clone $this->getRoutes(), $supportsRedirections)); + $code = rtrim($this->compileRoutes(clone $this->getRoutes(), $supportsRedirections), "\n"); return <<getIterator(); $keys = array_keys($routeIterator->getArrayCopy()); @@ -86,10 +87,11 @@ EOF; if ($route instanceof RouteCollection) { $prefix = $route->getPrefix(); $optimizable = $prefix && count($route->all()) > 1 && false === strpos($route->getPrefix(), '{'); - $indent = ''; + $optimized = false; + if ($optimizable) { for ($j = $i; $j < $keysCount; $j++) { - if ($keys[$j] === null) { + if (null === $keys[$j]) { continue; } @@ -113,28 +115,25 @@ EOF; } if ($prefix !== $parentPrefix) { - $code[] = sprintf(" if (0 === strpos(\$pathinfo, %s)) {", var_export($prefix, true)); - $indent = ' '; + $code .= sprintf(" if (0 === strpos(\$pathinfo, %s)) {\n", var_export($prefix, true)); + $optimized = true; } } - foreach ($this->compileRoutes($route, $supportsRedirections, $prefix) as $line) { - foreach (explode("\n", $line) as $l) { - if ($l) { - $code[] = $indent.$l; - } else { - $code[] = $l; + if ($optimized) { + foreach (explode("\n", $this->compileRoutes($route, $supportsRedirections, $prefix)) as $line) { + if ('' !== $line) { + $code .= ' '; // apply extra indention } + $code .= $line."\n"; } - } - - if ($optimizable && $prefix !== $parentPrefix) { - $code[] = " }\n"; + $code = substr($code, 0, -2); // remove redundant last two line breaks + $code .= " }\n\n"; + } else { + $code .= $this->compileRoutes($route, $supportsRedirections, $prefix); } } else { - foreach ($this->compileRoute($route, $name, $supportsRedirections, $parentPrefix) as $line) { - $code[] = $line; - } + $code .= $this->compileRoute($route, $name, $supportsRedirections, $parentPrefix)."\n"; } } @@ -143,12 +142,13 @@ EOF; private function compileRoute(Route $route, $name, $supportsRedirections, $parentPrefix = null) { - $code = array(); + $code = ''; $compiledRoute = $route->compile(); $conditions = array(); $hasTrailingSlash = false; $matches = false; $methods = array(); + if ($req = $route->getRequirement('_method')) { $methods = explode('|', strtoupper($req)); // GET and HEAD are equivalent @@ -156,6 +156,7 @@ EOF; $methods[] = 'HEAD'; } } + $supportsTrailingSlash = $supportsRedirections && (!$methods || in_array('HEAD', $methods)); if (!count($compiledRoute->getVariables()) && false !== preg_match('#^(.)\^(?P.*?)\$\1#', str_replace(array("\n", ' '), '', $compiledRoute->getRegex()), $m)) { @@ -182,39 +183,42 @@ EOF; $conditions = implode(' && ', $conditions); - $gotoname = 'not_'.preg_replace('/[^A-Za-z0-9_]/', '', $name); - - $code[] = <<context->getMethod() != '$methods[0]') { \$allow[] = '$methods[0]'; goto $gotoname; } + EOF; } else { - $methods = implode('\', \'', $methods); - $code[] = <<context->getMethod(), array('$methods'))) { \$allow = array_merge(\$allow, array('$methods')); goto $gotoname; } + EOF; } } if ($hasTrailingSlash) { - $code[] = sprintf(<<redirect(\$pathinfo.'/', '%s'); + return \$this->redirect(\$pathinfo.'/', '$name'); } -EOF - , $name); + +EOF; } if ($scheme = $route->getRequirement('_scheme')) { @@ -222,34 +226,32 @@ EOF throw new \LogicException('The "_scheme" requirement is only supported for URL matchers that implement RedirectableUrlMatcherInterface.'); } - $code[] = sprintf(<<context->getScheme() !== '$scheme') { - return \$this->redirect(\$pathinfo, '%s', '$scheme'); + return \$this->redirect(\$pathinfo, '$name', '$scheme'); } -EOF - , $name); + +EOF; } // optimize parameters array if (true === $matches && $compiledRoute->getDefaults()) { - $code[] = sprintf(" return array_merge(\$this->mergeDefaults(\$matches, %s), array('_route' => '%s'));" + $code .= sprintf(" return array_merge(\$this->mergeDefaults(\$matches, %s), array('_route' => '%s'));\n" , str_replace("\n", '', var_export($compiledRoute->getDefaults(), true)), $name); } elseif (true === $matches) { - $code[] = sprintf(" \$matches['_route'] = '%s';", $name); - $code[] = sprintf(" return \$matches;", $name); + $code .= sprintf(" \$matches['_route'] = '%s';\n", $name); + $code .= " return \$matches;\n"; } elseif ($compiledRoute->getDefaults()) { - $code[] = sprintf(' return %s;', str_replace("\n", '', var_export(array_merge($compiledRoute->getDefaults(), array('_route' => $name)), true))); + $code .= sprintf(" return %s;\n", str_replace("\n", '', var_export(array_merge($compiledRoute->getDefaults(), array('_route' => $name)), true))); } else { - $code[] = sprintf(" return array('_route' => '%s');", $name); + $code .= sprintf(" return array('_route' => '%s');\n", $name); } - $code[] = " }"; + $code .= " }\n"; if ($methods) { - $code[] = " $gotoname:"; + $code .= " $gotoname:\n"; } - $code[] = ''; - return $code; } diff --git a/src/Symfony/Component/Routing/RouteCollection.php b/src/Symfony/Component/Routing/RouteCollection.php index cc8fdc964c..400cfb8978 100644 --- a/src/Symfony/Component/Routing/RouteCollection.php +++ b/src/Symfony/Component/Routing/RouteCollection.php @@ -73,7 +73,7 @@ class RouteCollection implements \IteratorAggregate } /** - * Gets the current RouteCollection as an Iterator. + * Gets the current RouteCollection as an Iterator that includes all routes and child route collections. * * @return \ArrayIterator An \ArrayIterator interface */ 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 7932944a34..6367119def 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher1.php @@ -131,7 +131,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher $matches['_route'] = 'bar2'; return $matches; } - } // overriden @@ -149,7 +148,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher $matches['_route'] = 'foo4'; return $matches; } - } // foo3 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 b438ba243e..7a4d5b12a2 100644 --- a/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php +++ b/src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php @@ -137,7 +137,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec $matches['_route'] = 'bar2'; return $matches; } - } // overriden @@ -155,7 +154,6 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec $matches['_route'] = 'foo4'; return $matches; } - } // foo3 diff --git a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php index ebf6ab4f16..aa0b05ca0b 100644 --- a/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php +++ b/src/Symfony/Component/Routing/Tests/Matcher/Dumper/PhpMatcherDumperTest.php @@ -20,12 +20,12 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase { public function testDump() { - $dumper = new PhpMatcherDumper($this->getRouteCollection(), new RequestContext()); + $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.'); - $collection = $this->getRouteCollection(); - // force HTTPS redirection $collection->add('secure', new Route( '/secure',