Browse Source

[Routing] fixed route matching when the prefix contains variables

Fabien Potencier 14 years ago
parent
commit
c72537da6b

+ 4 - 2
src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

@@ -74,8 +74,10 @@ EOF;
         $code = array();
         foreach ($routes as $name => $route) {
             if ($route instanceof RouteCollection) {
+                $prefix = $route->getPrefix();
+                $optimizable = $prefix && count($route->all()) > 1 && false === strpos($route->getPrefix(), '{');
                 $indent = '';
-                if (count($route->all()) > 1 && $prefix = $route->getPrefix()) {
+                if ($optimizable) {
                     $code[] = sprintf("        if (0 === strpos(\$pathinfo, '%s')) {", $prefix);
                     $indent = '    ';
                 }
@@ -86,7 +88,7 @@ EOF;
                     }
                 }
 
-                if ($indent) {
+                if ($optimizable) {
                     $code[] = "        }\n";
                 }
             } else {

+ 1 - 1
src/Symfony/Component/Routing/Matcher/UrlMatcher.php

@@ -87,7 +87,7 @@ class UrlMatcher implements UrlMatcherInterface
     {
         foreach ($routes as $name => $route) {
             if ($route instanceof RouteCollection) {
-                if ($route->getPrefix() !== substr($pathinfo, 0, strlen($route->getPrefix()))) {
+                if (false === strpos($route->getPrefix(), '{') && $route->getPrefix() !== substr($pathinfo, 0, strlen($route->getPrefix()))) {
                     continue;
                 }
 

+ 12 - 0
tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher1.php

@@ -117,6 +117,18 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Matcher\UrlMatcher
     
         }
 
+        // foo
+        if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#x', $pathinfo, $matches)) {
+            $matches['_route'] = 'foo';
+            return $matches;
+        }
+
+        // bar
+        if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#x', $pathinfo, $matches)) {
+            $matches['_route'] = 'bar';
+            return $matches;
+        }
+
         throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();
     }
 }

+ 12 - 0
tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher2.php

@@ -129,6 +129,18 @@ class ProjectUrlMatcher extends Symfony\Tests\Component\Routing\Fixtures\Redirec
     
         }
 
+        // foo
+        if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<foo>[^/]+?)$#x', $pathinfo, $matches)) {
+            $matches['_route'] = 'foo';
+            return $matches;
+        }
+
+        // bar
+        if (preg_match('#^/(?P<_locale>[^/]+?)/b/(?P<bar>[^/]+?)$#x', $pathinfo, $matches)) {
+            $matches['_route'] = 'bar';
+            return $matches;
+        }
+
         // secure
         if ($pathinfo === '/secure') {
             if ($this->context->getScheme() !== 'https') {

+ 8 - 0
tests/Symfony/Tests/Component/Routing/Matcher/Dumper/PhpMatcherDumperTest.php

@@ -82,6 +82,14 @@ class PhpMatcherDumperTest extends \PHPUnit_Framework_TestCase
         $collection2->addCollection($collection1, '/b');
         $collection->addCollection($collection2, '/a');
 
+        // "dynamic" prefix
+        $collection1 = new RouteCollection();
+        $collection1->add('foo', new Route('/{foo}'));
+        $collection1->add('bar', new Route('/{bar}'));
+        $collection2 = new RouteCollection();
+        $collection2->addCollection($collection1, '/b');
+        $collection->addCollection($collection2, '/{_locale}');
+
         $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.');
 

+ 15 - 0
tests/Symfony/Tests/Component/Routing/Matcher/UrlMatcherTest.php

@@ -135,6 +135,21 @@ class UrlMatcherTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(array('_route' => 'foo', 'foo' => 'foo'), $matcher->match('/a/b/foo'));
     }
 
+    public function testMatchWithDynamicPrefix()
+    {
+        $collection1 = new RouteCollection();
+        $collection1->add('foo', new Route('/{foo}'));
+
+        $collection2 = new RouteCollection();
+        $collection2->addCollection($collection1, '/b');
+
+        $collection = new RouteCollection();
+        $collection->addCollection($collection2, '/{_locale}');
+
+        $matcher = new UrlMatcher($collection, new RequestContext(), array());
+        $this->assertEquals(array('_locale' => 'fr', '_route' => 'foo', 'foo' => 'foo'), $matcher->match('/fr/b/foo'));
+    }
+
     public function testMatchRegression()
     {
         $coll = new RouteCollection();