From 29bfa13ff0c217d59e34d4915033a8ed65bdd3e8 Mon Sep 17 00:00:00 2001 From: Tobias Schultze Date: Mon, 19 Nov 2012 21:41:59 +0100 Subject: [PATCH] small fix of #5984 when the container param is not set this can happen when the config for the router is unset, but this method does not need to depend on routing. reading an unset config would raise an exception. --- .../Controller/RedirectController.php | 28 +++++++-------- .../Controller/RedirectControllerTest.php | 34 +++++++++++-------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php index 174dc473d5..842a08b11a 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php +++ b/src/Symfony/Bundle/FrameworkBundle/Controller/RedirectController.php @@ -28,7 +28,7 @@ class RedirectController extends ContainerAware * It expects a route path parameter. * By default, the response status code is 301. * - * If the route empty, the status code will be 410. + * If the route is empty, the status code will be 410. * If the permanent path parameter is set, the status code will be 302. * * @param string $route The route pattern to redirect to @@ -56,11 +56,11 @@ class RedirectController extends ContainerAware * If the path is empty, the status code will be 410. * If the permanent flag is set, the status code will be 302. * - * @param string $path The path to redirect to - * @param Boolean $permanent Whether the redirect is permanent or not - * @param Boolean $scheme The URL scheme (null to keep the current one) - * @param integer $httpPort The HTTP port - * @param integer $httpsPort The HTTPS port + * @param string $path The path 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) + * @param integer|null $httpPort The HTTP port (null to keep the current one for the same scheme or the configured port in the container) + * @param integer|null $httpsPort The HTTPS port (null to keep the current one for the same scheme or the configured port in the container) * * @return Response A Response instance */ @@ -89,27 +89,25 @@ class RedirectController extends ContainerAware $port = ''; if ('http' === $scheme) { - if ($httpPort == null) { + if (null === $httpPort) { if ('http' === $request->getScheme()) { $httpPort = $request->getPort(); - } else { + } elseif ($this->container->hasParameter('request_listener.http_port')) { $httpPort = $this->container->getParameter('request_listener.http_port'); } } - - if ($httpPort != null && $httpPort != 80) { + if (null !== $httpPort && 80 != $httpPort) { $port = ":$httpPort"; } } elseif ('https' === $scheme) { - if ($httpsPort == null) { + if (null === $httpsPort) { if ('https' === $request->getScheme()) { $httpsPort = $request->getPort(); - } else { + } elseif ($this->container->hasParameter('request_listener.https_port')) { $httpsPort = $this->container->getParameter('request_listener.https_port'); - } + }; } - - if ($httpsPort != null && $httpsPort != 443) { + if (null !== $httpsPort && 443 != $httpsPort) { $port = ":$httpsPort"; } } diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php index b4f6f0f023..984810bd9b 100644 --- a/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/Controller/RedirectControllerTest.php @@ -75,9 +75,7 @@ class RedirectControllerTest extends TestCase $returnResponse = $controller->redirectAction($route, $permanent); - $this->assertInstanceOf('\Symfony\Component\HttpFoundation\Response', $returnResponse); - - $this->assertTrue($returnResponse->isRedirect($url)); + $this->assertRedirectUrl($returnResponse, $url); $this->assertEquals($expectedCode, $returnResponse->getStatusCode()); } @@ -104,9 +102,7 @@ class RedirectControllerTest extends TestCase $controller = new RedirectController(); $returnResponse = $controller->urlRedirectAction('http://foo.bar/'); - $this->assertInstanceOf('\Symfony\Component\HttpFoundation\Response', $returnResponse); - - $this->assertEquals('http://foo.bar/', $returnResponse->headers->get('Location')); + $this->assertRedirectUrl($returnResponse, 'http://foo.bar/'); $this->assertEquals(302, $returnResponse->getStatusCode()); } @@ -179,7 +175,7 @@ class RedirectControllerTest extends TestCase $this->assertRedirectUrl($returnValue, $expectedUrl); } - public function createRequestObject($scheme, $host, $port, $baseUrl) + private function createRequestObject($scheme, $host, $port, $baseUrl) { $request = $this->getMock('Symfony\Component\HttpFoundation\Request'); $request @@ -202,7 +198,7 @@ class RedirectControllerTest extends TestCase return $request; } - public function createRedirectController($request, $httpPort = null, $httpsPort = null) + private function createRedirectController(Request $request, $httpPort = null, $httpsPort = null) { $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerInterface'); $container @@ -210,16 +206,26 @@ class RedirectControllerTest extends TestCase ->method('get') ->with($this->equalTo('request')) ->will($this->returnValue($request)); - if ($httpPort != null) { + if (null !== $httpPort) { $container - ->expects($this->at(1)) + ->expects($this->once()) + ->method('hasParameter') + ->with($this->equalTo('request_listener.http_port')) + ->will($this->returnValue(true)); + $container + ->expects($this->once()) ->method('getParameter') ->with($this->equalTo('request_listener.http_port')) ->will($this->returnValue($httpPort)); } - if ($httpsPort != null) { + if (null !== $httpsPort) { $container - ->expects($this->at(1)) + ->expects($this->once()) + ->method('hasParameter') + ->with($this->equalTo('request_listener.https_port')) + ->will($this->returnValue(true)); + $container + ->expects($this->once()) ->method('getParameter') ->with($this->equalTo('request_listener.https_port')) ->will($this->returnValue($httpsPort)); @@ -231,8 +237,8 @@ class RedirectControllerTest extends TestCase return $controller; } - public function assertRedirectUrl($returnValue, $expectedUrl) + public function assertRedirectUrl(Response $returnResponse, $expectedUrl) { - $this->assertTrue($returnValue->isRedirect($expectedUrl), "Expected: $expectedUrl\nGot: ".$returnValue->headers->get('Location')); + $this->assertTrue($returnResponse->isRedirect($expectedUrl), "Expected: $expectedUrl\nGot: ".$returnResponse->headers->get('Location')); } }