[Routing] fixed ApacheUrlMatcher and ApachMatcherDumper classes that …

…did not take care of default parameters in urls.
symfony · Jul 13, 2012 · e9d799c · e9d799c
1 parent 3dc1fe0
commit e9d799c
Show file tree

Hide file tree

Showing 5 changed files with 123 additions and 13 deletions.
diff --git a/src/Symfony/Component/Routing/Matcher/ApacheUrlMatcher.php b/src/Symfony/Component/Routing/Matcher/ApacheUrlMatcher.php
@@ -34,6 +34,7 @@ class ApacheUrlMatcher extends UrlMatcher
     public function match($pathinfo)
     {
         $parameters = array();
+        $defaults = array();
         $allow = array();
         $match = false;
 
@@ -44,25 +45,28 @@ public function match($pathinfo)
                 $name = substr($name, 9);
             }
 
-            if (0 === strpos($name, '_ROUTING_')) {
+            if (0 === strpos($name, '_ROUTING_DEFAULTS_')) {
+                $name = substr($name, 18);
+                $defaults[$name] = $value;
+            } elseif (0 === strpos($name, '_ROUTING_')) {
                 $name = substr($name, 9);
+                if ('_route' == $name) {
+                    $match = true;
+                    $parameters[$name] = $value;
+                } elseif (0 === strpos($name, '_allow_')) {
+                    $allow[] = substr($name, 7);
+                } else {
+                    $parameters[$name] = $value;
+                }
             } else {
                 continue;
             }
 
-            if ('_route' == $name) {
-                $match = true;
-            } elseif (0 === strpos($name, '_allow_')) {
-                $allow[] = substr($name, 7);
-            } else {
-                $parameters[$name] = $value;
-            }
-
             unset($_SERVER[$key]);
         }
 
         if ($match) {
-            return $parameters;
+            return $this->mergeDefaults($parameters, $defaults);
         } elseif (0 < count($allow)) {
             throw new MethodNotAllowedException($allow);
         } else {

diff --git a/src/Symfony/Component/Routing/Matcher/Dumper/ApacheMatcherDumper.php b/src/Symfony/Component/Routing/Matcher/Dumper/ApacheMatcherDumper.php
@@ -65,7 +65,7 @@ public function dump(array $options = array())
                 $variables[] = 'E=_ROUTING_'.$variable.':%'.($i + 1);
             }
             foreach ($route->getDefaults() as $key => $value) {
-                $variables[] = 'E=_ROUTING_'.$key.':'.strtr($value, array(
+                $variables[] = 'E=_ROUTING_DEFAULTS_'.$key.':'.strtr($value, array(
                     ':'  => '\\:',
                     '='  => '\\=',
                     '\\' => '\\\\',

diff --git a/tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher1.apache b/tests/Symfony/Tests/Component/Routing/Fixtures/dumper/url_matcher1.apache
@@ -4,7 +4,11 @@ RewriteRule .* - [QSA,L]
 
 # foo
 RewriteCond %{REQUEST_URI} ^/foo/(baz|symfony)$
-RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foo,E=_ROUTING_bar:%1,E=_ROUTING_def:test]
+RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foo,E=_ROUTING_bar:%1,E=_ROUTING_DEFAULTS_def:test]
+
+# foobar
+RewriteCond %{REQUEST_URI} ^/foo(?:/([^/]+?))?$
+RewriteRule .* app.php [QSA,L,E=_ROUTING__route:foobar,E=_ROUTING_bar:%1,E=_ROUTING_DEFAULTS_bar:toto]
 
 # bar
 RewriteCond %{REQUEST_URI} ^/bar/([^/]+?)$
@@ -51,7 +55,7 @@ RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz5,E=_ROUTING_foo:%1]
 
 # baz6
 RewriteCond %{REQUEST_URI} ^/test/baz$
-RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz6,E=_ROUTING_foo:bar\ baz]
+RewriteRule .* app.php [QSA,L,E=_ROUTING__route:baz6,E=_ROUTING_DEFAULTS_foo:bar\ baz]
 
 # baz7
 RewriteCond %{REQUEST_URI} ^/te\ st/baz$

diff --git a/tests/Symfony/Tests/Component/Routing/Matcher/ApacheUrlMatcherTest.php b/tests/Symfony/Tests/Component/Routing/Matcher/ApacheUrlMatcherTest.php
@@ -0,0 +1,97 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Tests\Component\Routing\Matcher;
+
+use Symfony\Component\Routing\RouteCollection;
+use Symfony\Component\Routing\RequestContext;
+use Symfony\Component\Routing\Matcher\ApacheUrlMatcher;
+
+class ApacheUrlMatcherTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @dataProvider getMatchData
+     */
+    public function testMatch($name, $pathinfo, $server, $expect)
+    {
+        $collection = new RouteCollection();
+        $context = new RequestContext();
+        $matcher = new ApacheUrlMatcher($collection, $context);
+
+        $_SERVER = $server;
+
+        $result = $matcher->match($pathinfo, $server);
+        $this->assertSame(var_export($expect, true), var_export($result, true));
+    }
+
+    public function getMatchData()
+    {
+        return array(
+            array(
+                'Simple route',
+                '/hello/world',
+                array(
+                    '_ROUTING__route' => 'hello',
+                    '_ROUTING__controller' => 'AcmeBundle:Default:index',
+                    '_ROUTING_name' => 'world',
+                ),
+                array(
+                    '_route' => 'hello',
+                    '_controller' => 'AcmeBundle:Default:index',
+                    'name' => 'world',
+                ),
+            ),
+            array(
+                'Route with params and defaults',
+                '/hello/hugo',
+                array(
+                    '_ROUTING__route' => 'hello',
+                    '_ROUTING__controller' => 'AcmeBundle:Default:index',
+                    '_ROUTING_name' => 'hugo',
+                    '_ROUTING_DEFAULTS_name' => 'world',
+                ),
+                array(
+                    'name' => 'hugo',
+                    '_route' => 'hello',
+                    '_controller' => 'AcmeBundle:Default:index',
+                ),
+            ),
+            array(
+                'Route with defaults only',
+                '/hello',
+                array(
+                    '_ROUTING__route' => 'hello',
+                    '_ROUTING__controller' => 'AcmeBundle:Default:index',
+                    '_ROUTING_DEFAULTS_name' => 'world',
+                ),
+                array(
+                    'name' => 'world',
+                    '_route' => 'hello',
+                    '_controller' => 'AcmeBundle:Default:index',
+                ),
+            ),
+            array(
+                'REDIRECT_ envs',
+                '/hello/world',
+                array(
+                    'REDIRECT__ROUTING__route' => 'hello',
+                    'REDIRECT__ROUTING__controller' => 'AcmeBundle:Default:index',
+                    'REDIRECT__ROUTING_name' => 'world',
+                ),
+                array(
+                    '_route' => 'hello',
+                    '_controller' => 'AcmeBundle:Default:index',
+                    'name' => 'world',
+                ),
+            ),
+        );
+    }
+}
diff --git a/tests/Symfony/Tests/Component/Routing/Matcher/Dumper/ApacheMatcherDumperTest.php b/tests/Symfony/Tests/Component/Routing/Matcher/Dumper/ApacheMatcherDumperTest.php
@@ -70,6 +70,11 @@ private function getRouteCollection()
             array('def' => 'test'),
             array('bar' => 'baz|symfony')
         ));
+        // defaults parameters in pattern
+        $collection->add('foobar', new Route(
+            '/foo/{bar}',
+            array('bar' => 'toto')
+        ));
         // method requirement
         $collection->add('bar', new Route(
             '/bar/{foo}',