From 49062aacac286af27bd8b459f6c567f32ce91cf8 Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Sun, 7 Apr 2013 18:53:34 +0200 Subject: [PATCH 1/3] make it possible to ignore route attributes in the RedirectContoller --- .../Controller/RedirectController.php | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php index 7a255edc90..ceb1da61db 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php @@ -32,19 +32,26 @@ class RedirectController extends ContainerAware * In case the route name is empty, the status code will be 404 when permanent is false * and 410 otherwise. * - * @param string $route The route name to redirect to - * @param Boolean $permanent Whether the redirection is permanent + * @param string $route The route name to redirect to + * @param Boolean $permanent Whether the redirection is permanent + * @param Boolean|array $ignoreAttributes Whether to ignore attributes or an array of attributes to ignore * * @return Response A Response instance */ - public function redirectAction($route, $permanent = false) + public function redirectAction($route, $permanent = false, $ignoreAttributes = false) { if ('' == $route) { return new Response(null, $permanent ? 410 : 404); } - $attributes = $this->container->get('request')->attributes->get('_route_params'); - unset($attributes['route'], $attributes['permanent']); + $attributes = array(); + if (false === $ignoreAttributes || is_array($ignoreAttributes)) { + $attributes = $this->container->get('request')->attributes->get('_route_params'); + unset($attributes['route'], $attributes['permanent']); + if ($ignoreAttributes) { + $attributes = array_diff_key($attributes, array_flip($ignoreAttributes)); + } + } return new RedirectResponse($this->container->get('router')->generate($route, $attributes, UrlGeneratorInterface::ABSOLUTE_URL), $permanent ? 301 : 302); } From 0c7b65faf660849c30a60a2bb7060f61a83f3c3d Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Sat, 20 Apr 2013 22:34:45 +0200 Subject: [PATCH 2/3] inject the Request via the action signature, rather than fetching it from the DIC --- .../Controller/RedirectController.php | 10 +++-- .../Controller/RedirectControllerTest.php | 44 +++++++------------ 2 files changed, 23 insertions(+), 31 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php index ceb1da61db..59df4ae2a6 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php @@ -13,6 +13,7 @@ namespace Symfony\Bundle\FrameworkBundle\Controller; use Symfony\Component\DependencyInjection\ContainerAware; use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; @@ -32,13 +33,14 @@ class RedirectController extends ContainerAware * In case the route name is empty, the status code will be 404 when permanent is false * and 410 otherwise. * + * @param Request $request The request instance * @param string $route The route name to redirect to * @param Boolean $permanent Whether the redirection is permanent * @param Boolean|array $ignoreAttributes Whether to ignore attributes or an array of attributes to ignore * * @return Response A Response instance */ - public function redirectAction($route, $permanent = false, $ignoreAttributes = false) + public function redirectAction(Request $request, $route, $permanent = false, $ignoreAttributes = false) { if ('' == $route) { return new Response(null, $permanent ? 410 : 404); @@ -46,7 +48,7 @@ class RedirectController extends ContainerAware $attributes = array(); if (false === $ignoreAttributes || is_array($ignoreAttributes)) { - $attributes = $this->container->get('request')->attributes->get('_route_params'); + $attributes = $request->attributes->get('_route_params'); unset($attributes['route'], $attributes['permanent']); if ($ignoreAttributes) { $attributes = array_diff_key($attributes, array_flip($ignoreAttributes)); @@ -65,6 +67,7 @@ class RedirectController extends ContainerAware * In case the path is empty, the status code will be 404 when permanent is false * and 410 otherwise. * + * @param Request $request The request instance * @param string $path The absolute path or URL to redirect to * @param Boolean $permanent Whether the redirect is permanent or not * @param string|null $scheme The URL scheme (null to keep the current one) @@ -73,7 +76,7 @@ class RedirectController extends ContainerAware * * @return Response A Response instance */ - public function urlRedirectAction($path, $permanent = false, $scheme = null, $httpPort = null, $httpsPort = null) + public function urlRedirectAction(Request $request, $path, $permanent = false, $scheme = null, $httpPort = null, $httpsPort = null) { if ('' == $path) { return new Response(null, $permanent ? 410 : 404); @@ -86,7 +89,6 @@ class RedirectController extends ContainerAware return new RedirectResponse($path, $statusCode); } - $request = $this->container->get('request'); if (null === $scheme) { $scheme = $request->getScheme(); } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php index 171116585b..1aa32c8981 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php @@ -24,16 +24,14 @@ class RedirectControllerTest extends TestCase { public function testEmptyRoute() { - $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); - + $request = new Request(); $controller = new RedirectController(); - $controller->setContainer($container); - $returnResponse = $controller->redirectAction('', true); + $returnResponse = $controller->redirectAction($request, '', true); $this->assertInstanceOf('\Symfony\Component\HttpFoundation\Response', $returnResponse); $this->assertEquals(410, $returnResponse->getStatusCode()); - $returnResponse = $controller->redirectAction('', false); + $returnResponse = $controller->redirectAction($request, '', false); $this->assertInstanceOf('\Symfony\Component\HttpFoundation\Response', $returnResponse); $this->assertEquals(404, $returnResponse->getStatusCode()); } @@ -71,13 +69,7 @@ class RedirectControllerTest extends TestCase $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); $container - ->expects($this->at(0)) - ->method('get') - ->with($this->equalTo('request')) - ->will($this->returnValue($request)); - - $container - ->expects($this->at(1)) + ->expects($this->once()) ->method('get') ->with($this->equalTo('router')) ->will($this->returnValue($router)); @@ -85,7 +77,7 @@ class RedirectControllerTest extends TestCase $controller = new RedirectController(); $controller->setContainer($container); - $returnResponse = $controller->redirectAction($route, $permanent); + $returnResponse = $controller->redirectAction($request, $route, $permanent); $this->assertRedirectUrl($returnResponse, $url); $this->assertEquals($expectedCode, $returnResponse->getStatusCode()); @@ -101,21 +93,23 @@ class RedirectControllerTest extends TestCase public function testEmptyPath() { + $request = new Request(); $controller = new RedirectController(); - $returnResponse = $controller->urlRedirectAction('', true); + $returnResponse = $controller->urlRedirectAction($request, '', true); $this->assertInstanceOf('\Symfony\Component\HttpFoundation\Response', $returnResponse); $this->assertEquals(410, $returnResponse->getStatusCode()); - $returnResponse = $controller->urlRedirectAction('', false); + $returnResponse = $controller->urlRedirectAction($request, '', false); $this->assertInstanceOf('\Symfony\Component\HttpFoundation\Response', $returnResponse); $this->assertEquals(404, $returnResponse->getStatusCode()); } public function testFullURL() { + $request = new Request(); $controller = new RedirectController(); - $returnResponse = $controller->urlRedirectAction('http://foo.bar/'); + $returnResponse = $controller->urlRedirectAction($request, 'http://foo.bar/'); $this->assertRedirectUrl($returnResponse, 'http://foo.bar/'); $this->assertEquals(302, $returnResponse->getStatusCode()); @@ -131,14 +125,14 @@ class RedirectControllerTest extends TestCase $expectedUrl = "https://$host:$httpsPort$baseUrl$path"; $request = $this->createRequestObject('http', $host, $httpPort, $baseUrl); - $controller = $this->createRedirectController($request, null, $httpsPort); - $returnValue = $controller->urlRedirectAction($path, false, 'https'); + $controller = $this->createRedirectController(null, $httpsPort); + $returnValue = $controller->urlRedirectAction($request, $path, false, 'https'); $this->assertRedirectUrl($returnValue, $expectedUrl); $expectedUrl = "http://$host:$httpPort$baseUrl$path"; $request = $this->createRequestObject('https', $host, $httpPort, $baseUrl); - $controller = $this->createRedirectController($request, $httpPort); - $returnValue = $controller->urlRedirectAction($path, false, 'http'); + $controller = $this->createRedirectController($httpPort); + $returnValue = $controller->urlRedirectAction($request, $path, false, 'http'); $this->assertRedirectUrl($returnValue, $expectedUrl); } @@ -186,7 +180,7 @@ class RedirectControllerTest extends TestCase $request = $this->createRequestObject($requestScheme, $host, $requestPort, $baseUrl); $controller = $this->createRedirectController($request); - $returnValue = $controller->urlRedirectAction($path, false, $scheme, $httpPort, $httpsPort); + $returnValue = $controller->urlRedirectAction($request, $path, false, $scheme, $httpPort, $httpsPort); $this->assertRedirectUrl($returnValue, $expectedUrl); } @@ -213,14 +207,10 @@ class RedirectControllerTest extends TestCase return $request; } - private function createRedirectController(Request $request, $httpPort = null, $httpsPort = null) + private function createRedirectController($httpPort = null, $httpsPort = null) { $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); - $container - ->expects($this->at(0)) - ->method('get') - ->with($this->equalTo('request')) - ->will($this->returnValue($request)); + if (null !== $httpPort) { $container ->expects($this->once()) From 190abc18421fd9f9e5028506df577f9b258d72ff Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Sat, 20 Apr 2013 22:35:33 +0200 Subject: [PATCH 3/3] add unit tests for $ignoreAttributes --- .../Tests/Controller/RedirectControllerTest.php | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php index 1aa32c8981..6bd636ae2a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php @@ -39,13 +39,12 @@ class RedirectControllerTest extends TestCase /** * @dataProvider provider */ - public function testRoute($permanent, $expectedCode) + public function testRoute($permanent, $ignoreAttributes, $expectedCode, $expectedAttributes) { $request = new Request(); $route = 'new-route'; $url = '/redirect-url'; - $params = array('additional-parameter' => 'value'); $attributes = array( 'route' => $route, 'permanent' => $permanent, @@ -53,9 +52,9 @@ class RedirectControllerTest extends TestCase '_route_params' => array( 'route' => $route, 'permanent' => $permanent, + 'additional-parameter' => 'value', ), ); - $attributes['_route_params'] = $attributes['_route_params'] + $params; $request->attributes = new ParameterBag($attributes); @@ -63,7 +62,7 @@ class RedirectControllerTest extends TestCase $router ->expects($this->once()) ->method('generate') - ->with($this->equalTo($route), $this->equalTo($params)) + ->with($this->equalTo($route), $this->equalTo($expectedAttributes)) ->will($this->returnValue($url)); $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); @@ -77,7 +76,7 @@ class RedirectControllerTest extends TestCase $controller = new RedirectController(); $controller->setContainer($container); - $returnResponse = $controller->redirectAction($request, $route, $permanent); + $returnResponse = $controller->redirectAction($request, $route, $permanent, $ignoreAttributes); $this->assertRedirectUrl($returnResponse, $url); $this->assertEquals($expectedCode, $returnResponse->getStatusCode()); @@ -86,8 +85,10 @@ class RedirectControllerTest extends TestCase public function provider() { return array( - array(true, 301), - array(false, 302), + array(true, false, 301, array('additional-parameter' => 'value')), + array(false, false, 302, array('additional-parameter' => 'value')), + array(false, true, 302, array()), + array(false, array('additional-parameter'), 302, array()), ); } @@ -178,7 +179,7 @@ class RedirectControllerTest extends TestCase $expectedUrl = "$scheme://$host$expectedPort$baseUrl$path"; $request = $this->createRequestObject($requestScheme, $host, $requestPort, $baseUrl); - $controller = $this->createRedirectController($request); + $controller = $this->createRedirectController(); $returnValue = $controller->urlRedirectAction($request, $path, false, $scheme, $httpPort, $httpsPort); $this->assertRedirectUrl($returnValue, $expectedUrl);