From 2bfbe4176af90f246e67ce53f034704690d6bc7e Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Thu, 7 Jun 2018 10:06:45 +0200 Subject: [PATCH 1/9] Make redirect url fallback configurable --- Controller/ThemeController.php | 18 +++++++++++++----- DependencyInjection/Configuration.php | 1 + DependencyInjection/LiipThemeExtension.php | 2 +- Resources/config/controller.xml | 1 + 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/Controller/ThemeController.php b/Controller/ThemeController.php index f7e62ce..99cf585 100644 --- a/Controller/ThemeController.php +++ b/Controller/ThemeController.php @@ -12,6 +12,7 @@ namespace Liip\ThemeBundle\Controller; use Liip\ThemeBundle\ActiveTheme; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RedirectResponse; @@ -40,18 +41,25 @@ class ThemeController */ protected $cookieOptions; + /** + * @var ContainerInterface + */ + protected $container; + /** * Theme controller construct. * - * @param ActiveTheme $activeTheme active theme instance - * @param array $themes Available themes - * @param array|null $cookieOptions The options of the cookie we look for the theme to set + * @param ActiveTheme $activeTheme active theme instance + * @param array $themes Available themes + * @param array|null $cookieOptions The options of the cookie we look for the theme to set + * @param ContainerInterface $container */ - public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null) + public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null, ContainerInterface $container) { $this->activeTheme = $activeTheme; $this->themes = $themes; $this->cookieOptions = $cookieOptions; + $this->container = $container; } /** @@ -73,7 +81,7 @@ public function switchAction(Request $request) $this->activeTheme->setName($theme); - $url = $request->headers->get('Referer', '/'); + $url = $request->headers->get('Referer', $this->container->get('router')->generate($this->container->getParameter('liip_theme.redirect_fallback'))); $response = new RedirectResponse($url); if (!empty($this->cookieOptions)) { diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index e77bd9f..dd06152 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -41,6 +41,7 @@ public function getConfigTreeBuilder() ->prototype('scalar')->end() ->end() ->scalarNode('active_theme')->defaultNull()->end() + ->scalarNode('redirect_fallback')->defaultValue('homepage')->end() ->arrayNode('path_patterns') ->addDefaultsIfNotSet() ->children() diff --git a/DependencyInjection/LiipThemeExtension.php b/DependencyInjection/LiipThemeExtension.php index 4a048d0..5a52c1d 100644 --- a/DependencyInjection/LiipThemeExtension.php +++ b/DependencyInjection/LiipThemeExtension.php @@ -29,7 +29,7 @@ public function load(array $configs, ContainerBuilder $container) { $config = $this->processConfiguration(new Configuration(), $configs); - foreach (array('themes', 'active_theme', 'path_patterns', 'cache_warming') as $key) { + foreach (array('themes', 'active_theme', 'path_patterns', 'cache_warming', 'redirect_fallback') as $key) { $container->setParameter($this->getAlias().'.'.$key, $config[$key]); } diff --git a/Resources/config/controller.xml b/Resources/config/controller.xml index f027a9e..8812a4e 100755 --- a/Resources/config/controller.xml +++ b/Resources/config/controller.xml @@ -9,6 +9,7 @@ %liip_theme.themes% %liip_theme.cookie% + From e961202b5efe843ee9bdf7d8f1acf779b81d3888 Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Wed, 13 Jun 2018 16:07:39 +0200 Subject: [PATCH 2/9] Fix unit tests --- Controller/ThemeController.php | 27 +++++++++++++++++---------- Resources/config/controller.xml | 3 ++- Tests/UseCaseTest.php | 13 ++++++++++++- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/Controller/ThemeController.php b/Controller/ThemeController.php index 99cf585..b80a9e7 100644 --- a/Controller/ThemeController.php +++ b/Controller/ThemeController.php @@ -12,11 +12,11 @@ namespace Liip\ThemeBundle\Controller; use Liip\ThemeBundle\ActiveTheme; -use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Cookie; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; +use Symfony\Component\Routing\RouterInterface; /** * Theme controller. @@ -42,24 +42,31 @@ class ThemeController protected $cookieOptions; /** - * @var ContainerInterface + * @var RouterInterface */ - protected $container; + protected $router; + + /** + * @var string + */ + protected $defaultRoute; /** * Theme controller construct. * - * @param ActiveTheme $activeTheme active theme instance - * @param array $themes Available themes - * @param array|null $cookieOptions The options of the cookie we look for the theme to set - * @param ContainerInterface $container + * @param ActiveTheme $activeTheme active theme instance + * @param array $themes Available themes + * @param array|null $cookieOptions The options of the cookie we look for the theme to set + * @param RouterInterface $router + * @param string $defaultRoute */ - public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null, ContainerInterface $container) + public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null, RouterInterface $router, $defaultRoute = '/') { $this->activeTheme = $activeTheme; $this->themes = $themes; $this->cookieOptions = $cookieOptions; - $this->container = $container; + $this->router = $router; + $this->defaultRoute = $defaultRoute; } /** @@ -81,7 +88,7 @@ public function switchAction(Request $request) $this->activeTheme->setName($theme); - $url = $request->headers->get('Referer', $this->container->get('router')->generate($this->container->getParameter('liip_theme.redirect_fallback'))); + $url = $request->headers->get('Referer', $this->router->generate($this->defaultRoute)); $response = new RedirectResponse($url); if (!empty($this->cookieOptions)) { diff --git a/Resources/config/controller.xml b/Resources/config/controller.xml index 8812a4e..f50cbcb 100755 --- a/Resources/config/controller.xml +++ b/Resources/config/controller.xml @@ -9,7 +9,8 @@ %liip_theme.themes% %liip_theme.cookie% - + + %liip_theme.redirect_fallback% diff --git a/Tests/UseCaseTest.php b/Tests/UseCaseTest.php index 2fee13d..0eba32e 100644 --- a/Tests/UseCaseTest.php +++ b/Tests/UseCaseTest.php @@ -16,6 +16,8 @@ use Liip\ThemeBundle\EventListener\ThemeRequestListener; use Liip\ThemeBundle\Controller\ThemeController; use Liip\ThemeBundle\ActiveTheme; +use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\Router; /** * Bundle Functional tests. @@ -133,9 +135,13 @@ public function testThemeAction($config, $assertion, $hasAlreadyACookie = true) $request = $this->getMockRequest('cookie'); } + $router = $this->getMockBuilder(Router::class) + ->disableOriginalConstructor() + ->getMock(); + $controller = false; if ($config['load_controllers']) { - $controller = new ThemeController($activeTheme, $config['themes'], $config['cookie']); + $controller = new ThemeController($activeTheme, $config['themes'], $config['cookie'], $router, $config['redirect_fallback']); } $listener = new ThemeRequestListener($activeTheme, $config['cookie'], $device); @@ -178,6 +184,7 @@ public function dataProvider() // all-in Controller wins over Cookie and Autodetection array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), + 'redirect_fallback' => 'homepage', 'active_theme' => 'default', 'autodetect_theme' => true, 'load_controllers' => true, @@ -194,6 +201,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), + 'redirect_fallback' => 'homepage', 'active_theme' => 'default', 'autodetect_theme' => true, 'load_controllers' => true, @@ -210,6 +218,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), + 'redirect_fallback' => 'homepage', 'active_theme' => 'default', 'autodetect_theme' => true, 'load_controllers' => false, @@ -226,6 +235,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), + 'redirect_fallback' => 'homepage', 'active_theme' => 'default', 'autodetect_theme' => false, 'load_controllers' => true, @@ -242,6 +252,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), + 'redirect_fallback' => 'homepage', 'active_theme' => 'default', 'autodetect_theme' => false, 'load_controllers' => true, From 92ba4e6dae385d20b827c454ea110b24d60f1484 Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Wed, 13 Jun 2018 16:23:56 +0200 Subject: [PATCH 3/9] Use / as fallback when there is no route given --- Controller/ThemeController.php | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Controller/ThemeController.php b/Controller/ThemeController.php index b80a9e7..df055e5 100644 --- a/Controller/ThemeController.php +++ b/Controller/ThemeController.php @@ -47,7 +47,7 @@ class ThemeController protected $router; /** - * @var string + * @var string|null */ protected $defaultRoute; @@ -58,9 +58,9 @@ class ThemeController * @param array $themes Available themes * @param array|null $cookieOptions The options of the cookie we look for the theme to set * @param RouterInterface $router - * @param string $defaultRoute + * @param string|null $defaultRoute */ - public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null, RouterInterface $router, $defaultRoute = '/') + public function __construct(ActiveTheme $activeTheme, array $themes, array $cookieOptions = null, RouterInterface $router, $defaultRoute = null) { $this->activeTheme = $activeTheme; $this->themes = $themes; @@ -88,7 +88,13 @@ public function switchAction(Request $request) $this->activeTheme->setName($theme); - $url = $request->headers->get('Referer', $this->router->generate($this->defaultRoute)); + if ($defaultRoute) { + $url = $request->headers->get('Referer', $this->router->generate($this->defaultRoute)); + } + else { + $url = $request->headers->get('Referer', '/'); + } + $response = new RedirectResponse($url); if (!empty($this->cookieOptions)) { From 3b49d2c08817e58b1d4ad0754bf04204e1632482 Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Wed, 13 Jun 2018 16:29:18 +0200 Subject: [PATCH 4/9] =?UTF-8?q?Fix=20missing=20$this=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Controller/ThemeController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Controller/ThemeController.php b/Controller/ThemeController.php index df055e5..3c1a7a1 100644 --- a/Controller/ThemeController.php +++ b/Controller/ThemeController.php @@ -88,7 +88,7 @@ public function switchAction(Request $request) $this->activeTheme->setName($theme); - if ($defaultRoute) { + if ($this->defaultRoute) { $url = $request->headers->get('Referer', $this->router->generate($this->defaultRoute)); } else { From ef729080fb5f9130f9308a996a9430635c265506 Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Wed, 13 Jun 2018 16:33:13 +0200 Subject: [PATCH 5/9] Make statement CS compliant --- Controller/ThemeController.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Controller/ThemeController.php b/Controller/ThemeController.php index 3c1a7a1..f2c3205 100644 --- a/Controller/ThemeController.php +++ b/Controller/ThemeController.php @@ -90,8 +90,7 @@ public function switchAction(Request $request) if ($this->defaultRoute) { $url = $request->headers->get('Referer', $this->router->generate($this->defaultRoute)); - } - else { + } else { $url = $request->headers->get('Referer', '/'); } From 741ca1030ea52f95a3c298242dcc5bb06a370517 Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Wed, 13 Jun 2018 16:45:21 +0200 Subject: [PATCH 6/9] Remove redundant code --- Controller/ThemeController.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Controller/ThemeController.php b/Controller/ThemeController.php index f2c3205..2585c9a 100644 --- a/Controller/ThemeController.php +++ b/Controller/ThemeController.php @@ -88,12 +88,15 @@ public function switchAction(Request $request) $this->activeTheme->setName($theme); + if ($this->defaultRoute) { - $url = $request->headers->get('Referer', $this->router->generate($this->defaultRoute)); + $redirect = $this->router->generate($this->defaultRoute); } else { - $url = $request->headers->get('Referer', '/'); + $redirect = '/'; } + $url = $request->headers->get('Referer', $redirect); + $response = new RedirectResponse($url); if (!empty($this->cookieOptions)) { From ab6eb1421a6a54db22d7101869f824ab6a56ef29 Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Wed, 13 Jun 2018 20:57:52 +0200 Subject: [PATCH 7/9] Set default value of redirect_fallback to null --- DependencyInjection/Configuration.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index dd06152..c7f3e1d 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -41,7 +41,7 @@ public function getConfigTreeBuilder() ->prototype('scalar')->end() ->end() ->scalarNode('active_theme')->defaultNull()->end() - ->scalarNode('redirect_fallback')->defaultValue('homepage')->end() + ->scalarNode('redirect_fallback')->defaultNull()->end() ->arrayNode('path_patterns') ->addDefaultsIfNotSet() ->children() From be1e60d4a1117e700f878869bf05beafd1580725 Mon Sep 17 00:00:00 2001 From: Silvia Ulenberg Date: Fri, 15 Jun 2018 14:07:00 +0200 Subject: [PATCH 8/9] Draft for routing test --- Tests/UseCaseTest.php | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/Tests/UseCaseTest.php b/Tests/UseCaseTest.php index 0eba32e..aeaf623 100644 --- a/Tests/UseCaseTest.php +++ b/Tests/UseCaseTest.php @@ -16,8 +16,6 @@ use Liip\ThemeBundle\EventListener\ThemeRequestListener; use Liip\ThemeBundle\Controller\ThemeController; use Liip\ThemeBundle\ActiveTheme; -use Symfony\Component\Routing\Route; -use Symfony\Component\Routing\Router; /** * Bundle Functional tests. @@ -70,6 +68,24 @@ protected function getEventMock($request, $response, $type = 'Symfony\Component\ return $event; } + /** + * @return \Symfony\Bundle\FrameworkBundle\Routing\Router + */ + protected function getRouterMock() + { + $router = $this->getMockBuilder('Symfony\Component\Routing\Router') + ->disableOriginalConstructor() + ->setMethods(['generate', 'supports', 'exists']) + ->getMock(); + + $router->expects($this->any()) + ->method('generate') + ->with('test_route') + ->will($this->returnValue('/test_route')); + + return $router; + } + protected function getMockRequest($theme, $cookieReturnValue = 'cookie', $userAgent = 'autodetect') { $request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request') @@ -135,9 +151,7 @@ public function testThemeAction($config, $assertion, $hasAlreadyACookie = true) $request = $this->getMockRequest('cookie'); } - $router = $this->getMockBuilder(Router::class) - ->disableOriginalConstructor() - ->getMock(); + $router = $this->getRouterMock(); $controller = false; if ($config['load_controllers']) { @@ -155,6 +169,7 @@ public function testThemeAction($config, $assertion, $hasAlreadyACookie = true) $this->getMockRequest($assertion['themeAfterController']) ); $this->assertCookieValue($response, $assertion['themeAfterController']); + //$this->assertEquals($response->getTargetUrl(), $assertion['redirect']); } // onResponse will not set the cookie if the controller modified it @@ -184,7 +199,7 @@ public function dataProvider() // all-in Controller wins over Cookie and Autodetection array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), - 'redirect_fallback' => 'homepage', + 'redirect_fallback' => 'test_route', 'active_theme' => 'default', 'autodetect_theme' => true, 'load_controllers' => true, @@ -194,6 +209,7 @@ public function dataProvider() 'themeAfterKernelRequest' => 'cookie', 'themeAfterController' => 'controller', 'themeAfterKernelResponse' => 'controller', + 'redirect' => '/test_route', ), true, ), @@ -201,7 +217,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), - 'redirect_fallback' => 'homepage', + 'redirect_fallback' => 'test_route', 'active_theme' => 'default', 'autodetect_theme' => true, 'load_controllers' => true, @@ -211,6 +227,7 @@ public function dataProvider() 'themeAfterKernelRequest' => 'autodetect', 'themeAfterController' => 'controller', 'themeAfterKernelResponse' => 'controller', + 'redirect' => '/test_route', ), false, ), @@ -218,7 +235,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), - 'redirect_fallback' => 'homepage', + 'redirect_fallback' => 'test_route', 'active_theme' => 'default', 'autodetect_theme' => true, 'load_controllers' => false, @@ -228,6 +245,7 @@ public function dataProvider() 'themeAfterKernelRequest' => 'autodetect', 'themeAfterController' => 'autodetect', 'themeAfterKernelResponse' => 'autodetect', + 'redirect' => '/test_route', ), false, ), @@ -235,7 +253,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), - 'redirect_fallback' => 'homepage', + 'redirect_fallback' => 'test_route', 'active_theme' => 'default', 'autodetect_theme' => false, 'load_controllers' => true, @@ -245,6 +263,7 @@ public function dataProvider() 'themeAfterKernelRequest' => 'default', 'themeAfterController' => 'controller', 'themeAfterKernelResponse' => 'controller', + 'redirect' => '/test_route', ), false, ), @@ -252,7 +271,7 @@ public function dataProvider() array( array( 'themes' => array('default', 'controller', 'cookie', 'autodetect'), - 'redirect_fallback' => 'homepage', + 'redirect_fallback' => 'test_route', 'active_theme' => 'default', 'autodetect_theme' => false, 'load_controllers' => true, @@ -262,6 +281,7 @@ public function dataProvider() 'themeAfterKernelRequest' => 'cookie', 'themeAfterController' => 'controller', 'themeAfterKernelResponse' => 'controller', + 'redirect' => '/test_route', ), true, ), From 7537a58c1b84b4b51e64f3276bc507de701d744d Mon Sep 17 00:00:00 2001 From: Lukas Kahwe Smith Date: Sat, 16 Jun 2018 09:26:37 +0200 Subject: [PATCH 9/9] fixed tests by removing the Request mock --- Controller/ThemeController.php | 4 +-- Tests/UseCaseTest.php | 47 +++++----------------------------- 2 files changed, 9 insertions(+), 42 deletions(-) diff --git a/Controller/ThemeController.php b/Controller/ThemeController.php index 2585c9a..5c876ca 100644 --- a/Controller/ThemeController.php +++ b/Controller/ThemeController.php @@ -90,9 +90,9 @@ public function switchAction(Request $request) if ($this->defaultRoute) { - $redirect = $this->router->generate($this->defaultRoute); + $redirect = $this->router->generate($this->defaultRoute); } else { - $redirect = '/'; + $redirect = '/'; } $url = $request->headers->get('Referer', $redirect); diff --git a/Tests/UseCaseTest.php b/Tests/UseCaseTest.php index aeaf623..ab4f214 100644 --- a/Tests/UseCaseTest.php +++ b/Tests/UseCaseTest.php @@ -86,41 +86,6 @@ protected function getRouterMock() return $router; } - protected function getMockRequest($theme, $cookieReturnValue = 'cookie', $userAgent = 'autodetect') - { - $request = $this->getMockBuilder('Symfony\Component\HttpFoundation\Request') - ->disableOriginalConstructor() - ->getMock(); - - $request->expects($this->any()) - ->method('get') - ->will($this->returnValue($theme)); - $request->cookies = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag') - ->disableOriginalConstructor() - ->getMock(); - $request->cookies->expects($this->any()) - ->method('get') - ->will($this->returnValue($cookieReturnValue)); - $request->headers = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag') - ->disableOriginalConstructor() - ->getMock(); - $request->headers->expects($this->any()) - ->method('get') - ->will($this->returnValue('/')); - $request->headers->expects($this->any()) - ->method('get') - ->will($this->returnValue($cookieReturnValue)); - - $request->server = $this->getMockBuilder('Symfony\Component\HttpFoundation\ParameterBag') - ->disableOriginalConstructor() - ->getMock(); - $request->server->expects($this->any()) - ->method('get') - ->will($this->returnValue($userAgent)); - - return $request; - } - private function getCookieValueFromResponse($response) { $cookies = $response->headers->getCookies(); @@ -146,9 +111,10 @@ public function testThemeAction($config, $assertion, $hasAlreadyACookie = true) } $response = new \Symfony\Component\HttpFoundation\Response(); $request = new \Symfony\Component\HttpFoundation\Request(); - if ($hasAlreadyACookie) { - $request = $this->getMockRequest('cookie'); + $request->query->set('theme', 'cookie'); + $request->cookies->set('cookieName', 'cookie'); + $request->server->set('HTTP_USER_AGENT', 'autodetect'); } $router = $this->getRouterMock(); @@ -165,11 +131,13 @@ public function testThemeAction($config, $assertion, $hasAlreadyACookie = true) $this->assertEquals($activeTheme->getName(), $assertion['themeAfterKernelRequest']); if ($controller) { + $request->query->set('theme', $assertion['themeAfterController']); + $response = $controller->switchAction( - $this->getMockRequest($assertion['themeAfterController']) + $request ); $this->assertCookieValue($response, $assertion['themeAfterController']); - //$this->assertEquals($response->getTargetUrl(), $assertion['redirect']); + $this->assertEquals($response->getTargetUrl(), $assertion['redirect']); } // onResponse will not set the cookie if the controller modified it @@ -285,7 +253,6 @@ public function dataProvider() ), true, ), - ); } }