feature #23440 [Routing] Add matched and default parameters to redirect responses (artursvonda, Tobion)

This PR was merged into the 3.4 branch.

Discussion
----------

[Routing] Add matched and default parameters to redirect responses

| Q | A |
| --- | --- |
| Branch? | master |
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #18012, #19037 |
| License | MIT |
| Doc PR |  |

Finished #18012 and #19037

Commits
-------

4c1fdd4 [Routing] also add matched params for redirect due to trailing slash
dc3f7a9 [Routing] Add matched and default parameters to redirect responses

src/Symfony/Bundle/FrameworkBundle/Tests/Routing/RedirectableUrlMatcherTest.php

@@ -33,7 +33,7 @@ class RedirectableUrlMatcherTest extends TestCase
                 'scheme' => null,
                 'httpPort' => $context->getHttpPort(),
                 'httpsPort' => $context->getHttpsPort(),
-                '_route' => null,
+                '_route' => 'foo',
             ),
             $matcher->match('/foo')
         );

src/Symfony/Component/Routing/CHANGELOG.md

@@ -5,6 +5,7 @@ CHANGELOG
 -----
 
  * Added support for prioritized routing loaders.
+ * Add matched and default parameters to redirect responses
 
 3.3.0
 -----

src/Symfony/Component/Routing/Matcher/Dumper/PhpMatcherDumper.php

@@ -333,10 +333,35 @@ EOF;
             }
         }
 
+        // the offset where the return value is appended below, with indendation
+        $retOffset = 12 + strlen($code);
+
+        // optimize parameters array
+        if ($matches || $hostMatches) {
+            $vars = array();
+            if ($hostMatches) {
+                $vars[] = '$hostMatches';
+            }
+            if ($matches) {
+                $vars[] = '$matches';
+            }
+            $vars[] = "array('_route' => '$name')";
+
+            $code .= sprintf(
+                "            \$ret = \$this->mergeDefaults(array_replace(%s), %s);\n",
+                implode(', ', $vars),
+                str_replace("\n", '', var_export($route->getDefaults(), true))
+            );
+        } elseif ($route->getDefaults()) {
+            $code .= sprintf("            \$ret = %s;\n", str_replace("\n", '', var_export(array_replace($route->getDefaults(), array('_route' => $name)), true)));
+        } else {
+            $code .= sprintf("            \$ret = array('_route' => '%s');\n", $name);
+        }
+
         if ($hasTrailingSlash) {
             $code .= <<<EOF
             if (substr(\$pathinfo, -1) !== '/') {
-                return \$this->redirect(\$pathinfo.'/', '$name');
+                return array_replace(\$ret, \$this->redirect(\$pathinfo.'/', '$name'));
             }
 
 
@@ -351,33 +376,17 @@ EOF;
             $code .= <<<EOF
             \$requiredSchemes = $schemes;
             if (!isset(\$requiredSchemes[\$scheme])) {
-                return \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes));
+                return array_replace(\$ret, \$this->redirect(\$pathinfo, '$name', key(\$requiredSchemes)));
             }
 
 
 EOF;
         }
 
-        // optimize parameters array
+        if ($hasTrailingSlash || $schemes) {
-        if ($matches || $hostMatches) {
+            $code .= "            return \$ret;\n";
-            $vars = array();
-            if ($hostMatches) {
-                $vars[] = '$hostMatches';
-            }
-            if ($matches) {
-                $vars[] = '$matches';
-            }
-            $vars[] = "array('_route' => '$name')";
-
-            $code .= sprintf(
-                "            return \$this->mergeDefaults(array_replace(%s), %s);\n",
-                implode(', ', $vars),
-                str_replace("\n", '', var_export($route->getDefaults(), true))
-            );
-        } elseif ($route->getDefaults()) {
-            $code .= sprintf("            return %s;\n", str_replace("\n", '', var_export(array_replace($route->getDefaults(), array('_route' => $name)), true)));
         } else {
-            $code .= sprintf("            return array('_route' => '%s');\n", $name);
+            $code = substr_replace($code, 'return', $retOffset, 6);
         }
         $code .= "        }\n";
 

src/Symfony/Component/Routing/Matcher/RedirectableUrlMatcher.php

@@ -32,9 +32,9 @@ abstract class RedirectableUrlMatcher extends UrlMatcher implements Redirectable
             }
 
             try {
-                parent::match($pathinfo.'/');
+                $parameters = parent::match($pathinfo.'/');
 
-                return $this->redirect($pathinfo.'/', null);
+                return array_replace($parameters, $this->redirect($pathinfo.'/', isset($parameters['_route']) ? $parameters['_route'] : null));
             } catch (ResourceNotFoundException $e2) {
                 throw $e;
             }

src/Symfony/Component/Routing/Matcher/UrlMatcher.php

@@ -163,15 +163,11 @@ class UrlMatcher implements UrlMatcherInterface, RequestMatcherInterface
 
             $status = $this->handleRouteRequirements($pathinfo, $name, $route);
 
-            if (self::ROUTE_MATCH === $status[0]) {
-                return $status[1];
-            }
-
             if (self::REQUIREMENT_MISMATCH === $status[0]) {
                 continue;
             }
 
-            return $this->getAttributes($route, $name, array_replace($matches, $hostMatches));
+            return $this->getAttributes($route, $name, array_replace($matches, $hostMatches, isset($status[1]) ? $status[1] : array()));
         }
     }
 

src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher2.php

@@ -87,22 +87,24 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
 
                 // baz3
                 if ('/test/baz3' === $trimmedPathinfo) {
+                    $ret = array('_route' => 'baz3');
                     if (substr($pathinfo, -1) !== '/') {
-                        return $this->redirect($pathinfo.'/', 'baz3');
+                        return array_replace($ret, $this->redirect($pathinfo.'/', 'baz3'));
                     }
 
-                    return array('_route' => 'baz3');
+                    return $ret;
                 }
 
             }
 
             // baz4
             if (preg_match('#^/test/(?P<foo>[^/]++)/?$#s', $pathinfo, $matches)) {
+                $ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'baz4')), array ());
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'baz4');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'baz4'));
                 }
 
-                return $this->mergeDefaults(array_replace($matches, array('_route' => 'baz4')), array ());
+                return $ret;
             }
 
             // baz5
@@ -181,11 +183,12 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
 
             // hey
             if ('/multi/hey' === $trimmedPathinfo) {
+                $ret = array('_route' => 'hey');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'hey');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'hey'));
                 }
 
-                return array('_route' => 'hey');
+                return $ret;
             }
 
             // overridden2
@@ -326,22 +329,24 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
 
         // secure
         if ('/secure' === $pathinfo) {
+            $ret = array('_route' => 'secure');
             $requiredSchemes = array (  'https' => 0,);
             if (!isset($requiredSchemes[$scheme])) {
-                return $this->redirect($pathinfo, 'secure', key($requiredSchemes));
+                return array_replace($ret, $this->redirect($pathinfo, 'secure', key($requiredSchemes)));
             }
 
-            return array('_route' => 'secure');
+            return $ret;
         }
 
         // nonsecure
         if ('/nonsecure' === $pathinfo) {
+            $ret = array('_route' => 'nonsecure');
             $requiredSchemes = array (  'http' => 0,);
             if (!isset($requiredSchemes[$scheme])) {
-                return $this->redirect($pathinfo, 'nonsecure', key($requiredSchemes));
+                return array_replace($ret, $this->redirect($pathinfo, 'nonsecure', key($requiredSchemes)));
             }
 
-            return array('_route' => 'nonsecure');
+            return $ret;
         }
 
         throw 0 < count($allow) ? new MethodNotAllowedException(array_unique($allow)) : new ResourceNotFoundException();

src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher5.php

@@ -61,29 +61,32 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
         if (0 === strpos($pathinfo, '/a')) {
             // a_fourth
             if ('/a/44' === $trimmedPathinfo) {
+                $ret = array('_route' => 'a_fourth');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'a_fourth');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'a_fourth'));
                 }
 
-                return array('_route' => 'a_fourth');
+                return $ret;
             }
 
             // a_fifth
             if ('/a/55' === $trimmedPathinfo) {
+                $ret = array('_route' => 'a_fifth');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'a_fifth');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'a_fifth'));
                 }
 
-                return array('_route' => 'a_fifth');
+                return $ret;
             }
 
             // a_sixth
             if ('/a/66' === $trimmedPathinfo) {
+                $ret = array('_route' => 'a_sixth');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'a_sixth');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'a_sixth'));
                 }
 
-                return array('_route' => 'a_sixth');
+                return $ret;
             }
 
         }
@@ -96,29 +99,32 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
         if (0 === strpos($pathinfo, '/nested/group')) {
             // nested_a
             if ('/nested/group/a' === $trimmedPathinfo) {
+                $ret = array('_route' => 'nested_a');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'nested_a');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'nested_a'));
                 }
 
-                return array('_route' => 'nested_a');
+                return $ret;
             }
 
             // nested_b
             if ('/nested/group/b' === $trimmedPathinfo) {
+                $ret = array('_route' => 'nested_b');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'nested_b');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'nested_b'));
                 }
 
-                return array('_route' => 'nested_b');
+                return $ret;
             }
 
             // nested_c
             if ('/nested/group/c' === $trimmedPathinfo) {
+                $ret = array('_route' => 'nested_c');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'nested_c');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'nested_c'));
                 }
 
-                return array('_route' => 'nested_c');
+                return $ret;
             }
 
         }
@@ -126,29 +132,32 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
         elseif (0 === strpos($pathinfo, '/slashed/group')) {
             // slashed_a
             if ('/slashed/group' === $trimmedPathinfo) {
+                $ret = array('_route' => 'slashed_a');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'slashed_a');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'slashed_a'));
                 }
 
-                return array('_route' => 'slashed_a');
+                return $ret;
             }
 
             // slashed_b
             if ('/slashed/group/b' === $trimmedPathinfo) {
+                $ret = array('_route' => 'slashed_b');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'slashed_b');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'slashed_b'));
                 }
 
-                return array('_route' => 'slashed_b');
+                return $ret;
             }
 
             // slashed_c
             if ('/slashed/group/c' === $trimmedPathinfo) {
+                $ret = array('_route' => 'slashed_c');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'slashed_c');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'slashed_c'));
                 }
 
-                return array('_route' => 'slashed_c');
+                return $ret;
             }
 
         }

src/Symfony/Component/Routing/Tests/Fixtures/dumper/url_matcher7.php

@@ -38,11 +38,12 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
         if (0 === strpos($pathinfo, '/trailing/simple')) {
             // simple_trailing_slash_no_methods
             if ('/trailing/simple/no-methods' === $trimmedPathinfo) {
+                $ret = array('_route' => 'simple_trailing_slash_no_methods');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'simple_trailing_slash_no_methods');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'simple_trailing_slash_no_methods'));
                 }
 
-                return array('_route' => 'simple_trailing_slash_no_methods');
+                return $ret;
             }
 
             // simple_trailing_slash_GET_method
@@ -52,11 +53,12 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
                     goto not_simple_trailing_slash_GET_method;
                 }
 
+                $ret = array('_route' => 'simple_trailing_slash_GET_method');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'simple_trailing_slash_GET_method');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'simple_trailing_slash_GET_method'));
                 }
 
-                return array('_route' => 'simple_trailing_slash_GET_method');
+                return $ret;
             }
             not_simple_trailing_slash_GET_method:
 
@@ -67,11 +69,12 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
                     goto not_simple_trailing_slash_HEAD_method;
                 }
 
+                $ret = array('_route' => 'simple_trailing_slash_HEAD_method');
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'simple_trailing_slash_HEAD_method');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'simple_trailing_slash_HEAD_method'));
                 }
 
-                return array('_route' => 'simple_trailing_slash_HEAD_method');
+                return $ret;
             }
             not_simple_trailing_slash_HEAD_method:
 
@@ -91,11 +94,12 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
         elseif (0 === strpos($pathinfo, '/trailing/regex')) {
             // regex_trailing_slash_no_methods
             if (0 === strpos($pathinfo, '/trailing/regex/no-methods') && preg_match('#^/trailing/regex/no\\-methods/(?P<param>[^/]++)/?$#s', $pathinfo, $matches)) {
+                $ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'regex_trailing_slash_no_methods')), array ());
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'regex_trailing_slash_no_methods');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'regex_trailing_slash_no_methods'));
                 }
 
-                return $this->mergeDefaults(array_replace($matches, array('_route' => 'regex_trailing_slash_no_methods')), array ());
+                return $ret;
             }
 
             // regex_trailing_slash_GET_method
@@ -105,11 +109,12 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
                     goto not_regex_trailing_slash_GET_method;
                 }
 
+                $ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'regex_trailing_slash_GET_method')), array ());
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'regex_trailing_slash_GET_method');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'regex_trailing_slash_GET_method'));
                 }
 
-                return $this->mergeDefaults(array_replace($matches, array('_route' => 'regex_trailing_slash_GET_method')), array ());
+                return $ret;
             }
             not_regex_trailing_slash_GET_method:
 
@@ -120,11 +125,12 @@ class ProjectUrlMatcher extends Symfony\Component\Routing\Tests\Fixtures\Redirec
                     goto not_regex_trailing_slash_HEAD_method;
                 }
 
+                $ret = $this->mergeDefaults(array_replace($matches, array('_route' => 'regex_trailing_slash_HEAD_method')), array ());
                 if (substr($pathinfo, -1) !== '/') {
-                    return $this->redirect($pathinfo.'/', 'regex_trailing_slash_HEAD_method');
+                    return array_replace($ret, $this->redirect($pathinfo.'/', 'regex_trailing_slash_HEAD_method'));
                 }
 
-                return $this->mergeDefaults(array_replace($matches, array('_route' => 'regex_trailing_slash_HEAD_method')), array ());
+                return $ret;
             }
             not_regex_trailing_slash_HEAD_method:
 

src/Symfony/Component/Routing/Tests/Matcher/RedirectableUrlMatcherTest.php

@@ -24,7 +24,7 @@ class RedirectableUrlMatcherTest extends TestCase
         $coll->add('foo', new Route('/foo/'));
 
         $matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
-        $matcher->expects($this->once())->method('redirect');
+        $matcher->expects($this->once())->method('redirect')->will($this->returnValue(array()));
         $matcher->match('/foo');
     }
 
@@ -65,8 +65,37 @@ class RedirectableUrlMatcherTest extends TestCase
         $matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
         $matcher
             ->expects($this->never())
-            ->method('redirect')
+            ->method('redirect');
-        ;
         $matcher->match('/foo');
     }
+
+    public function testSchemeRedirectWithParams()
+    {
+        $coll = new RouteCollection();
+        $coll->add('foo', new Route('/foo/{bar}', array(), array(), array(), '', array('https')));
+
+        $matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
+        $matcher
+            ->expects($this->once())
+            ->method('redirect')
+            ->with('/foo/baz', 'foo', 'https')
+            ->will($this->returnValue(array('redirect' => 'value')))
+        ;
+        $this->assertEquals(array('_route' => 'foo', 'bar' => 'baz', 'redirect' => 'value'), $matcher->match('/foo/baz'));
+    }
+
+    public function testSlashRedirectWithParams()
+    {
+        $coll = new RouteCollection();
+        $coll->add('foo', new Route('/foo/{bar}/'));
+
+        $matcher = $this->getMockForAbstractClass('Symfony\Component\Routing\Matcher\RedirectableUrlMatcher', array($coll, new RequestContext()));
+        $matcher
+            ->expects($this->once())
+            ->method('redirect')
+            ->with('/foo/baz/', 'foo', null)
+            ->will($this->returnValue(array('redirect' => 'value')))
+        ;
+        $this->assertEquals(array('_route' => 'foo', 'bar' => 'baz', 'redirect' => 'value'), $matcher->match('/foo/baz'));
+    }
 }