From 8c512b0d2d73a0930a420030dd6fecb8cb2d506f Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 18 Jan 2013 14:00:40 -0500 Subject: [PATCH 01/11] [ticket/11334] Properly generate controller URL until paths issue gets fixed PHPBB3-11334 --- phpBB/includes/controller/helper.php | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 6cacc8fefa..2098f51edf 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -85,17 +85,14 @@ class phpbb_controller_helper } /** - * Easily generate a URL + * Generate a URL * - * @param array $url_parts Each array element is a 'folder' - * i.e. array('my', 'ext') maps to ./app.php/my/ext - * @param mixed $query The Query string, passed directly into the second - * argument of append_sid() - * @return string A URL that has already been run through append_sid() + * @param string $route The route to travel + * @return string The URL already passed through append_sid() */ - public function url(array $url_parts, $query = '') + protected function url($route) { - return append_sid($this->phpbb_root_path . implode('/', $url_parts), $query); + return append_sid($this->phpbb_root_path . 'app.' . $this->php_ext, array('controller' => $route)); } /** From d3e2fae66d74f79ef7dcfe2e24f47efaa5c106e2 Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 15 Feb 2013 16:48:43 -0500 Subject: [PATCH 02/11] [ticket/11334] Add a test for the controller helper URL method PHPBB3-11334 --- phpBB/includes/controller/helper.php | 2 +- tests/controller/controller_test.php | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 2098f51edf..0e64829874 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -55,7 +55,7 @@ class phpbb_controller_helper * @param string $phpbb_root_path phpBB root path * @param string $php_ext PHP extension */ - public function __construct(phpbb_template $template, phpbb_user $user, $phpbb_root_path, $php_ext) + public function __construct(phpbb_template $template = null, phpbb_user $user = null, $phpbb_root_path = './', $php_ext = '.php') { $this->template = $template; $this->user = $user; diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 198fb3c6dd..97f6d6152a 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -40,6 +40,12 @@ class phpbb_controller_test extends phpbb_test_case $this->assertEquals(2, sizeof($routes)); } + public function test_controller_url_helper() + { + $helper = new phpbb_controller_helper; + $this->assertEquals($helper->url('foo/bar'),'./app.php?controller=foo/bar'); + } + public function test_controller_resolver() { $container = new ContainerBuilder(); From 5e89ce1898857f29e5345adf31d62bbed1fb985b Mon Sep 17 00:00:00 2001 From: David King Date: Fri, 15 Feb 2013 16:52:54 -0500 Subject: [PATCH 03/11] [ticket/11334] Make url helper method public PHPBB3-11334 --- phpBB/includes/controller/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 0e64829874..f2beca2056 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -90,7 +90,7 @@ class phpbb_controller_helper * @param string $route The route to travel * @return string The URL already passed through append_sid() */ - protected function url($route) + public function url($route) { return append_sid($this->phpbb_root_path . 'app.' . $this->php_ext, array('controller' => $route)); } From 48aefb13b0f66dd84f5e7a2f18ca485fabe4833b Mon Sep 17 00:00:00 2001 From: David King Date: Sat, 16 Feb 2013 19:18:16 -0500 Subject: [PATCH 04/11] [ticket/11334] Make $phpbb_dispatcher global, as done in append_sid test PHPBB3-11334 --- tests/controller/controller_test.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 97f6d6152a..c4818dbd6a 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -42,6 +42,9 @@ class phpbb_controller_test extends phpbb_test_case public function test_controller_url_helper() { + global $phpbb_dispatcher; + + $phpbb_dispatcher = new phpbb_mock_event_dispatcher; $helper = new phpbb_controller_helper; $this->assertEquals($helper->url('foo/bar'),'./app.php?controller=foo/bar'); } From 5850a2cbf6e8313feeb55154e1083d73b45f4dc3 Mon Sep 17 00:00:00 2001 From: David King Date: Sat, 16 Feb 2013 19:21:34 -0500 Subject: [PATCH 05/11] [ticket/11334] Remove extraneous period PHPBB3-11334 --- phpBB/includes/controller/helper.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index f2beca2056..451c448a18 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -92,7 +92,7 @@ class phpbb_controller_helper */ public function url($route) { - return append_sid($this->phpbb_root_path . 'app.' . $this->php_ext, array('controller' => $route)); + return append_sid($this->phpbb_root_path . 'app' . $this->php_ext, array('controller' => $route)); } /** From cd697e68129868dc811c141660652360e0f0f983 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 15 Mar 2013 13:35:00 +0100 Subject: [PATCH 06/11] [ticket/11334] Include functions.php and fix class name in tests PHPBB3-11334 --- tests/controller/controller_test.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index c4818dbd6a..8cdedb27e2 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -7,6 +7,8 @@ * */ +require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; + use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -14,7 +16,7 @@ use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; -class phpbb_controller_test extends phpbb_test_case +class phpbb_controller_controller_test extends phpbb_test_case { public function setUp() { From ff9a0e4ef4756c5a9cce3f023b07d9f9a0e5653a Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 15 Mar 2013 13:35:43 +0100 Subject: [PATCH 07/11] [ticket/11334] Expand functionality of helper->url() Expanded the functionality of helper->url() to support all parameters of append_sid() itself. PHPBB3-11334 --- phpBB/includes/controller/helper.php | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 451c448a18..4c021849f4 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -87,12 +87,30 @@ class phpbb_controller_helper /** * Generate a URL * - * @param string $route The route to travel + * @param string $route The route to travel + * @param mixed $params String or array of additional url parameters + * @param bool $is_amp Is url using & (true) or & (false) + * @param string $session_id Possibility to use a custom session id instead of the global one * @return string The URL already passed through append_sid() */ - public function url($route) + public function url($route, $params = false, $is_amp = true, $session_id = false) { - return append_sid($this->phpbb_root_path . 'app' . $this->php_ext, array('controller' => $route)); + if (is_array($params) && !empty($params)) + { + $params = array_merge(array( + 'controller' => $route, + ), $params); + } + else if (is_string($params) && $params) + { + $params = 'controller=' . $route . (($is_amp) ? '&' : '&') . $params; + } + else + { + $params = array('controller' => $route); + } + + return append_sid($this->phpbb_root_path . 'app' . $this->php_ext, $params, $is_amp, $session_id); } /** From 9259e635cacce34699115b909f256f9302ec3aaa Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 15 Mar 2013 14:01:15 +0100 Subject: [PATCH 08/11] [ticket/11334] Move unit tests for helper->url() into own file PHPBB3-11334 --- tests/controller/controller_test.php | 11 ------ tests/controller/helper_url_test.php | 55 ++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 tests/controller/helper_url_test.php diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php index 8cdedb27e2..c06bf7d548 100644 --- a/tests/controller/controller_test.php +++ b/tests/controller/controller_test.php @@ -7,8 +7,6 @@ * */ -require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php'; - use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -42,15 +40,6 @@ class phpbb_controller_controller_test extends phpbb_test_case $this->assertEquals(2, sizeof($routes)); } - public function test_controller_url_helper() - { - global $phpbb_dispatcher; - - $phpbb_dispatcher = new phpbb_mock_event_dispatcher; - $helper = new phpbb_controller_helper; - $this->assertEquals($helper->url('foo/bar'),'./app.php?controller=foo/bar'); - } - public function test_controller_resolver() { $container = new ContainerBuilder(); diff --git a/tests/controller/helper_url_test.php b/tests/controller/helper_url_test.php new file mode 100644 index 0000000000..6e3f535cf3 --- /dev/null +++ b/tests/controller/helper_url_test.php @@ -0,0 +1,55 @@ + 1, 'f' => 2), true, false, 'app.php?controller=foo/bar&t=1&f=2', 'parameters in params-argument as array'), + + // Custom sid parameter + array('foo/bar', 't=1&f=2', true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid', 'using session_id'), + + // Testing anchors + // Unsupported: array('foo/bar?t=1&f=2#anchor', false, true, false, 'app.php?controller=foo/bar&t=1&f=2#anchor', 'anchor in url-argument'), + array('foo/bar', 't=1&f=2#anchor', true, false, 'app.php?controller=foo/bar&t=1&f=2#anchor', 'anchor in params-argument'), + array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php?controller=foo/bar&t=1&f=2#anchor', 'anchor in params-argument (array)'), + + // Anchors and custom sid + // Unsupported: array('foo/bar?t=1&f=2#anchor', false, true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid#anchor', 'anchor in url-argument using session_id'), + array('foo/bar', 't=1&f=2#anchor', true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument using session_id'), + array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'), + + // Empty parameters should not append the & + array('foo/bar', false, true, false, 'app.php?controller=foo/bar', 'no params using bool false'), + array('foo/bar', '', true, false, 'app.php?controller=foo/bar', 'no params using empty string'), + array('foo/bar', array(), true, false, 'app.php?controller=foo/bar', 'no params using empty array'), + ); + } + + /** + * @dataProvider helper_url_data + */ + public function test_helper_url($route, $params, $is_amp, $session_id, $expected, $description) + { + global $phpbb_dispatcher; + + $phpbb_dispatcher = new phpbb_mock_event_dispatcher; + $helper = new phpbb_controller_helper; + $this->assertEquals($helper->url($route, $params, $is_amp, $session_id), $expected); + } +} + From 076711d9a95e05083143b7ac4a589914a2e2b2ad Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 15 Mar 2013 14:02:46 +0100 Subject: [PATCH 09/11] [ticket/11334] Use mocks instead of making parameters optional PHPBB3-11334 --- phpBB/includes/controller/helper.php | 2 +- tests/controller/helper_url_test.php | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 4c021849f4..1464267711 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -55,7 +55,7 @@ class phpbb_controller_helper * @param string $phpbb_root_path phpBB root path * @param string $php_ext PHP extension */ - public function __construct(phpbb_template $template = null, phpbb_user $user = null, $phpbb_root_path = './', $php_ext = '.php') + public function __construct(phpbb_template $template, phpbb_user $user, $phpbb_root_path, $php_ext) { $this->template = $template; $this->user = $user; diff --git a/tests/controller/helper_url_test.php b/tests/controller/helper_url_test.php index 6e3f535cf3..7754278219 100644 --- a/tests/controller/helper_url_test.php +++ b/tests/controller/helper_url_test.php @@ -48,7 +48,11 @@ class phpbb_controller_helper_url_test extends phpbb_test_case global $phpbb_dispatcher; $phpbb_dispatcher = new phpbb_mock_event_dispatcher; - $helper = new phpbb_controller_helper; + $this->style_resource_locator = new phpbb_style_resource_locator(); + $this->user = $this->getMock('phpbb_user'); + $this->template = new phpbb_template($phpbb_root_path, $phpEx, $config, $this->user, $this->style_resource_locator, new phpbb_template_context()); + + $helper = new phpbb_controller_helper($this->template, $this->user, '', '.php'); $this->assertEquals($helper->url($route, $params, $is_amp, $session_id), $expected); } } From 9157095cdabe629765ab0583f2850c2d4771c1d1 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 15 Mar 2013 14:06:56 +0100 Subject: [PATCH 10/11] [ticket/11334] Fix copyright year in test file PHPBB3-11334 --- tests/controller/helper_url_test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/controller/helper_url_test.php b/tests/controller/helper_url_test.php index 7754278219..f27c03501d 100644 --- a/tests/controller/helper_url_test.php +++ b/tests/controller/helper_url_test.php @@ -2,7 +2,7 @@ /** * * @package testing -* @copyright (c) 2011 phpBB Group +* @copyright (c) 2013 phpBB Group * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2 * */ From 3b0cdc53629c3a852762ae9b96b809cf4b1af2c4 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Fri, 15 Mar 2013 15:21:15 +0100 Subject: [PATCH 11/11] [ticket/11334] Allow parameters to be specified in the route PHPBB3-11334 --- phpBB/includes/controller/helper.php | 9 ++++++++- tests/controller/helper_url_test.php | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/phpBB/includes/controller/helper.php b/phpBB/includes/controller/helper.php index 1464267711..46c6307cb4 100644 --- a/phpBB/includes/controller/helper.php +++ b/phpBB/includes/controller/helper.php @@ -95,6 +95,13 @@ class phpbb_controller_helper */ public function url($route, $params = false, $is_amp = true, $session_id = false) { + $route_params = ''; + if (($route_delim = strpos($route, '?')) !== false) + { + $route_params = substr($route, $route_delim); + $route = substr($route, 0, $route_delim); + } + if (is_array($params) && !empty($params)) { $params = array_merge(array( @@ -110,7 +117,7 @@ class phpbb_controller_helper $params = array('controller' => $route); } - return append_sid($this->phpbb_root_path . 'app' . $this->php_ext, $params, $is_amp, $session_id); + return append_sid($this->phpbb_root_path . 'app' . $this->php_ext . $route_params, $params, $is_amp, $session_id); } /** diff --git a/tests/controller/helper_url_test.php b/tests/controller/helper_url_test.php index f27c03501d..195f48d8a9 100644 --- a/tests/controller/helper_url_test.php +++ b/tests/controller/helper_url_test.php @@ -15,7 +15,7 @@ class phpbb_controller_helper_url_test extends phpbb_test_case public function helper_url_data() { return array( - // Unsupported: array('foo/bar?t=1&f=2', false, true, false, 'app.php?controller=foo/bar&t=1&f=2', 'parameters in url-argument'), + array('foo/bar?t=1&f=2', false, true, false, 'app.php?t=1&f=2&controller=foo/bar', 'parameters in url-argument'), array('foo/bar', 't=1&f=2', true, false, 'app.php?controller=foo/bar&t=1&f=2', 'parameters in params-argument using amp'), array('foo/bar', 't=1&f=2', false, false, 'app.php?controller=foo/bar&t=1&f=2', 'parameters in params-argument using &'), array('foo/bar', array('t' => 1, 'f' => 2), true, false, 'app.php?controller=foo/bar&t=1&f=2', 'parameters in params-argument as array'), @@ -24,12 +24,12 @@ class phpbb_controller_helper_url_test extends phpbb_test_case array('foo/bar', 't=1&f=2', true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid', 'using session_id'), // Testing anchors - // Unsupported: array('foo/bar?t=1&f=2#anchor', false, true, false, 'app.php?controller=foo/bar&t=1&f=2#anchor', 'anchor in url-argument'), + array('foo/bar?t=1&f=2#anchor', false, true, false, 'app.php?t=1&f=2&controller=foo/bar#anchor', 'anchor in url-argument'), array('foo/bar', 't=1&f=2#anchor', true, false, 'app.php?controller=foo/bar&t=1&f=2#anchor', 'anchor in params-argument'), array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php?controller=foo/bar&t=1&f=2#anchor', 'anchor in params-argument (array)'), // Anchors and custom sid - // Unsupported: array('foo/bar?t=1&f=2#anchor', false, true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid#anchor', 'anchor in url-argument using session_id'), + array('foo/bar?t=1&f=2#anchor', false, true, 'custom-sid', 'app.php?t=1&f=2&controller=foo/bar&sid=custom-sid#anchor', 'anchor in url-argument using session_id'), array('foo/bar', 't=1&f=2#anchor', true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument using session_id'), array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'app.php?controller=foo/bar&t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),