Merge pull request #2092 from nickvergessen/ticket/12090

[ticket/12090] Pass route name to url() instead of the url itself

phpBB/config/services.yml

@@ -94,6 +94,7 @@ services:
             - @template
             - @user
             - @config
+            - @controller.provider
             - %core.root_path%
             - %core.php_ext%
 
@@ -104,6 +105,13 @@ services:
             - @service_container
             - @template
 
+    controller.provider:
+        class: phpbb\controller\provider
+        arguments:
+            - @ext.finder
+        calls:
+            - [find, [%core.root_path%]]
+
     cron.task_collection:
         class: phpbb\di\service_collection
         arguments:
@@ -262,6 +270,7 @@ services:
         arguments:
             - @template
             - @user
+            - @controller.helper
 
     path_helper:
         class: phpbb\path_helper

phpBB/includes/functions_url_matcher.php

@@ -53,8 +53,8 @@ function phpbb_get_url_matcher(\phpbb\extension\finder $finder, RequestContext $
 */
 function phpbb_create_dumped_url_matcher(\phpbb\extension\finder $finder, $root_path, $php_ext)
 {
-	$provider = new \phpbb\controller\provider();
+	$provider = new \phpbb\controller\provider($finder);
-	$routes = $provider->import_paths_from_finder($finder)->find($root_path);
+	$routes = $provider->find($root_path)->get_routes();
 	$dumper = new PhpMatcherDumper($routes);
 	$cached_url_matcher_dump = $dumper->dump(array(
 		'class'			=> 'phpbb_url_matcher',
@@ -72,8 +72,8 @@ function phpbb_create_dumped_url_matcher(\phpbb\extension\finder $finder, $root_
 */
 function phpbb_create_url_matcher(\phpbb\extension\finder $finder, RequestContext $context, $root_path)
 {
-	$provider = new \phpbb\controller\provider();
+	$provider = new \phpbb\controller\provider($finder);
-	$routes = $provider->import_paths_from_finder($finder)->find($root_path);
+	$routes = $provider->find($root_path)->get_routes();
 	return new UrlMatcher($routes, $context);
 }
 

phpBB/phpbb/controller/helper.php

@@ -10,6 +10,8 @@
 namespace phpbb\controller;
 
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Routing\Generator\UrlGenerator;
+use Symfony\Component\Routing\RequestContext;
 
 /**
 * Controller helper class, contains methods that do things for controllers
@@ -53,16 +55,18 @@ class helper
 	* @param \phpbb\template\template $template Template object
 	* @param \phpbb\user $user User object
 	* @param \phpbb\config\config $config Config object
+	* @param \phpbb\controller\provider $provider Path provider
 	* @param string $phpbb_root_path phpBB root path
 	* @param string $php_ext PHP extension
 	*/
-	public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, $phpbb_root_path, $php_ext)
+	public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\config\config $config, \phpbb\controller\provider $provider, $phpbb_root_path, $php_ext)
 	{
 		$this->template = $template;
 		$this->user = $user;
 		$this->config = $config;
 		$this->phpbb_root_path = $phpbb_root_path;
 		$this->php_ext = $php_ext;
+		$this->route_collection = $provider->get_routes();
 	}
 
 	/**
@@ -87,21 +91,33 @@ class helper
 	}
 
 	/**
-	* Generate a URL
+	* Generate a URL to a route
 	*
-	* @param string	$route		The route to travel
+	* @param string	$route		Name of the route to travel
-	* @param mixed	$params		String or array of additional url parameters
+	* @param array	$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, $params = false, $is_amp = true, $session_id = false)
+	public function route($route, array $params = array(), $is_amp = true, $session_id = false)
 	{
-		$route_params = '';
+		$anchor = '';
-		if (($route_delim = strpos($route, '?')) !== false)
+		if (isset($params['#']))
 		{
-			$route_params = substr($route, $route_delim);
+			$anchor = '#' . $params['#'];
-			$route = substr($route, 0, $route_delim);
+			unset($params['#']);
+		}
+		$url_generator = new UrlGenerator($this->route_collection, new RequestContext());
+		$route_url = $url_generator->generate($route, $params);
+
+		if (strpos($route_url, '/') === 0)
+		{
+			$route_url = substr($route_url, 1);
+		}
+
+		if ($is_amp)
+		{
+			$route_url = str_replace(array('&', '&'), array('&', '&'), $route_url);
 		}
 
 		// If enable_mod_rewrite is false, we need to include app.php
@@ -111,7 +127,7 @@ class helper
 			$route_prefix .= 'app.' . $this->php_ext . '/';
 		}
 
-		return append_sid($route_prefix . "$route" . $route_params, $params, $is_amp, $session_id);
+		return append_sid($route_prefix . $route_url . $anchor, false, $is_amp, $session_id);
 	}
 
 	/**

phpBB/phpbb/controller/provider.php

@@ -25,51 +25,59 @@ class provider
 	*/
 	protected $routing_files;
 
+	/**
+	* Collection of the routes in phpBB and all found extensions
+	* @var RouteCollection
+	*/
+	protected $routes;
+
 	/**
 	* Construct method
 	*
 	* @param array() $routing_files Array of strings containing paths
 	*							to YAML files holding route information
 	*/
-	public function __construct($routing_files = array())
+	public function __construct(\phpbb\extension\finder $finder = null, $routing_files = array())
 	{
 		$this->routing_files = $routing_files;
-	}
 
-	/**
+		if ($finder)
-	* Locate paths containing routing files
-	* This sets an internal property but does not return the paths.
-	*
-	* @return The current instance of this object for method chaining
-	*/
-	public function import_paths_from_finder(\phpbb\extension\finder $finder)
 		{
 			// We hardcode the path to the core config directory
 			// because the finder cannot find it
-		$this->routing_files = array_merge(array('config/routing.yml'), array_keys($finder
+			$this->routing_files = array_merge($this->routing_files, array('config/routing.yml'), array_keys($finder
 					->directory('config')
 					->suffix('routing.yml')
 					->find()
 			));
+		}
+	}
+
+	/**
+	* Find a list of controllers and return it
+	*
+	* @param string $base_path Base path to prepend to file paths
+	* @return null
+	*/
+	public function find($base_path = '')
+	{
+		$this->routes = new RouteCollection;
+		foreach ($this->routing_files as $file_path)
+		{
+			$loader = new YamlFileLoader(new FileLocator($base_path));
+			$this->routes->addCollection($loader->load($file_path));
+		}
 
 		return $this;
 	}
 
 	/**
-	* Get a list of controllers and return it
+	* Get the list of routes
 	*
-	* @param string $base_path Base path to prepend to file paths
+	* @return RouteCollection Get the route collection
-	* @return array Array of controllers and their route information
 	*/
-	public function find($base_path = '')
+	public function get_routes()
 	{
-		$routes = new RouteCollection;
+		return $this->routes;
-		foreach ($this->routing_files as $file_path)
-		{
-			$loader = new YamlFileLoader(new FileLocator($base_path));
-			$routes->addCollection($loader->load($file_path));
-		}
-
-		return $routes;
 	}
 }

phpBB/phpbb/pagination.php

@@ -22,11 +22,13 @@ class pagination
 	*
 	* @param	\phpbb\template\template	$template
 	* @param	\phpbb\user					$user
+	* @param	\phpbb\controller\helper	$helper
 	*/
-	public function __construct(\phpbb\template\template $template, \phpbb\user $user)
+	public function __construct(\phpbb\template\template $template, \phpbb\user $user, \phpbb\controller\helper $helper)
 	{
 		$this->template = $template;
 		$this->user = $user;
+		$this->helper = $helper;
 	}
 
 	/**
@@ -44,9 +46,26 @@ class pagination
 	*/
 	protected function generate_page_link($base_url, $on_page, $start_name, $per_page)
 	{
-		if (strpos($start_name, '%d') !== false)
+		if (!is_string($base_url))
 		{
-			return ($on_page > 1) ? sprintf($base_url, (int) $on_page) : str_replace($start_name, '', $base_url);
+			if (is_array($base_url['routes']))
+			{
+				$route = ($on_page > 1) ? $base_url['routes'][1] : $base_url['routes'][0];
+			}
+			else
+			{
+				$route = $base_url['routes'];
+			}
+			$params = (isset($base_url['params'])) ? $base_url['params'] : array();
+			$is_amp = (isset($base_url['is_amp'])) ? $base_url['is_amp'] : true;
+			$session_id = (isset($base_url['session_id'])) ? $base_url['session_id'] : false;
+
+			if ($on_page > 1 || !is_array($base_url['routes']))
+			{
+				$params[$start_name] = (int) $on_page;
+			}
+
+			return $this->helper->route($route, $params, $is_amp, $session_id);
 		}
 		else
 		{
@@ -194,7 +213,8 @@ class pagination
 		$tpl_prefix = ($tpl_prefix == 'PAGINATION') ? '' : $tpl_prefix . '_';
 
 		$template_array = array(
-			$tpl_prefix . 'BASE_URL'		=> $base_url,
+			$tpl_prefix . 'BASE_URL'		=> is_string($base_url) ? $base_url : '',//@todo: Fix this for routes
+			$tpl_prefix . 'START_NAME'		=> $start_name,
 			$tpl_prefix . 'PER_PAGE'		=> $per_page,
 			'U_' . $tpl_prefix . 'PREVIOUS_PAGE'	=> ($on_page != 1) ? $u_previous_page : '',
 			'U_' . $tpl_prefix . 'NEXT_PAGE'		=> ($on_page != $total_pages) ? $u_next_page : '',

phpBB/styles/prosilver/template/forum_fn.js

@@ -37,17 +37,14 @@ function jumpto(item) {
 		on_page = item.attr('data-on-page'),
 		per_page = item.attr('data-per-page'),
 		base_url = item.attr('data-base-url'),
+		start_name = item.attr('data-start-name'),
 		page = prompt(jump_page, on_page);
 
 	if (page !== null && !isNaN(page) && page == Math.floor(page) && page > 0) {
-		if (base_url.indexOf('%d') === -1) {
 		if (base_url.indexOf('?') === -1) {
-				document.location.href = base_url + '?start=' + ((page - 1) * per_page);
+			document.location.href = base_url + '?' + start_name + '=' + ((page - 1) * per_page);
 		} else {
-				document.location.href = base_url.replace(/&/g, '&') + '&start=' + ((page - 1) * per_page);
+			document.location.href = base_url.replace(/&/g, '&') + '&' + start_name + '=' + ((page - 1) * per_page);
-			}
-		} else {
-			document.location.href = base_url.replace('%d', page);
 		}
 	}
 }

phpBB/styles/prosilver/template/pagination.html

@@ -1,4 +1,8 @@
-					<a href="#" class="pagination-trigger" title="{L_JUMP_TO_PAGE}" data-lang-jump-page="{L_JUMP_PAGE|e('html_attr')}{L_COLON}" data-on-page="{CURRENT_PAGE}" data-per-page="{PER_PAGE}" data-base-url="{BASE_URL|e('html_attr')}">{PAGE_NUMBER}</a> &bull;
+					<!-- IF BASE_URL -->
+					<a href="#" class="pagination-trigger" title="{L_JUMP_TO_PAGE}" data-lang-jump-page="{L_JUMP_PAGE|e('html_attr')}{L_COLON}" data-on-page="{CURRENT_PAGE}" data-per-page="{PER_PAGE}" data-base-url="{BASE_URL|e('html_attr')}" data-base-is-route="{BASE_IS_ROUTE}" data-start-name="{START_NAME}">{PAGE_NUMBER}</a> &bull;
+					<!-- ELSE -->
+					{PAGE_NUMBER} &bull;
+					<!-- ENDIF -->
 					<ul>
 					<!-- BEGIN pagination -->
 						<!-- IF pagination.S_IS_PREV -->

phpBB/styles/subsilver2/template/pagination.html

@@ -1,5 +1,5 @@
 <!-- IF .pagination -->
-	<b><a href="#" onclick="jumpto(); return false;" title="{L_JUMP_TO_PAGE}">{L_GOTO_PAGE}</a> 
+	<!-- IF BASE_URL --><b><a href="#" onclick="jumpto(); return false;" title="{L_JUMP_TO_PAGE}">{L_GOTO_PAGE}</a></b><!-- ENDIF -->
 	<!-- BEGIN pagination -->
 		<!-- IF pagination.S_IS_PREV --><a href="{pagination.PAGE_URL}">{L_PREVIOUS}</a>
 		<!-- ELSEIF pagination.S_IS_CURRENT --><strong>{pagination.PAGE_NUMBER}</strong>

tests/controller/controller_test.php

@@ -21,20 +21,18 @@ class phpbb_controller_controller_test extends phpbb_test_case
 		$this->extension_manager = new phpbb_mock_extension_manager(
 			dirname(__FILE__) . '/',
 			array(
-				'foo' => array(
+				'vendor2/foo' => array(
-					'ext_name' => 'foo',
+					'ext_name' => 'vendor2/foo',
 					'ext_active' => '1',
-					'ext_path' => 'ext/foo/',
+					'ext_path' => 'ext/vendor2/foo/',
 				),
 			));
 	}
 
 	public function test_provider()
 	{
-		$provider = new \phpbb\controller\provider;
+		$provider = new \phpbb\controller\provider($this->extension_manager->get_finder());
-		$routes = $provider
+		$routes = $provider->find(__DIR__)->get_routes();
-			->import_paths_from_finder($this->extension_manager->get_finder())
-			->find(__DIR__);
 
 		// This will need to be updated if any new routes are defined
 		$this->assertInstanceOf('Symfony\Component\Routing\Route', $routes->get('core_controller'));
@@ -52,7 +50,7 @@ class phpbb_controller_controller_test extends phpbb_test_case
 		$container = new ContainerBuilder();
 		// YamlFileLoader only uses one path at a time, so we need to loop
 		// through all of the ones we are using.
-		foreach (array(__DIR__.'/config', __DIR__.'/ext/foo/config') as $path)
+		foreach (array(__DIR__.'/config', __DIR__ . '/ext/vendor2/foo/config') as $path)
 		{
 			$loader = new YamlFileLoader($container, new FileLocator($path));
 			$loader->load('services.yml');
@@ -60,9 +58,9 @@ class phpbb_controller_controller_test extends phpbb_test_case
 
 		// Autoloading classes within the tests folder does not work
 		// so I'll include them manually.
-		if (!class_exists('foo\\controller'))
+		if (!class_exists('vendor2\\foo\\controller'))
 		{
-			include(__DIR__.'/ext/foo/controller.php');
+			include(__DIR__ . '/ext/vendor2/foo/controller.php');
 		}
 		if (!class_exists('phpbb\\controller\\foo'))
 		{

tests/controller/ext/foo/config/routing_2.yml

@@ -1,3 +0,0 @@
-controller2:
-    pattern: /bar
-    defaults: { _controller: foo.controller:handle }

tests/controller/ext/vendor2/foo/config/routing_2.yml

@@ -0,0 +1,6 @@
+controller2:
+    pattern: /bar
+    defaults: { _controller: foo.controller:handle }
+controller3:
+    pattern: /bar/p-{p}
+    defaults: { _controller: foo.controller:handle }

tests/controller/helper_route_test.php

@@ -0,0 +1,128 @@
+<?php
+/**
+*
+* @package testing
+* @copyright (c) 2013 phpBB Group
+* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
+*
+*/
+
+require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
+
+class phpbb_controller_helper_route_test extends phpbb_test_case
+{
+	public function setUp()
+	{
+		global $phpbb_dispatcher, $phpbb_root_path, $phpEx;
+
+		$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
+		$this->user = $this->getMock('\phpbb\user');
+		$phpbb_path_helper = new \phpbb\path_helper(
+			new \phpbb\symfony_request(
+				new phpbb_mock_request()
+			),
+			new \phpbb\filesystem(),
+			$phpbb_root_path,
+			$phpEx
+		);
+		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
+		$this->template = new phpbb\template\twig\twig($phpbb_path_helper, $this->config, $this->user, new \phpbb\template\context());
+
+		$finder = new \phpbb\extension\finder(
+			new phpbb_mock_extension_manager(
+				dirname(__FILE__) . '/',
+				array(
+					'vendor2/foo' => array(
+						'ext_name' => 'vendor2/foo',
+						'ext_active' => '1',
+						'ext_path' => 'ext/vendor2/foo/',
+					),
+				)
+			),
+			new \phpbb\filesystem(),
+			dirname(__FILE__) . '/',
+			new phpbb_mock_cache()
+		);
+		$this->provider = new \phpbb\controller\provider($finder);
+		$this->provider->find(dirname(__FILE__) . '/');
+	}
+
+	public function helper_url_data_no_rewrite()
+	{
+		return array(
+			array('controller2', array('t' => 1, 'f' => 2), true, false, 'app.php/foo/bar?t=1&amp;f=2', 'parameters in params-argument as array'),
+			array('controller2', array('t' => 1, 'f' => 2), false, false, 'app.php/foo/bar?t=1&f=2', 'parameters in params-argument as array'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, false, 'app.php/foo/bar/p-3?t=1&amp;f=2', 'parameters in params-argument as array'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2), false, false, 'app.php/foo/bar/p-3?t=1&f=2', 'parameters in params-argument as array'),
+
+			// Custom sid parameter
+			array('controller2', array('t' => 1, 'f' => 2), true, 'custom-sid', 'app.php/foo/bar?t=1&amp;f=2&amp;sid=custom-sid', 'params-argument (array) using session_id'),
+			array('controller2', array('t' => 1, 'f' => 2), false, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, 'custom-sid', 'app.php/foo/bar/p-3?t=1&amp;f=2&amp;sid=custom-sid', 'params-argument (array) using session_id'),
+
+			// Testing anchors
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php/foo/bar?t=1&amp;f=2#anchor', 'anchor in params-argument (array)'),
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, false, 'app.php/foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php/foo/bar/p-3?t=1&amp;f=2#anchor', 'anchor in params-argument (array)'),
+
+			// Anchors and custom sid
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'app.php/foo/bar?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, 'custom-sid', 'app.php/foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'app.php/foo/bar/p-3?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
+
+			// Empty parameters should not append the &amp; or ?
+			array('controller2', array(), true, false, 'app.php/foo/bar', 'no params using empty array'),
+			array('controller2', array(), false, false, 'app.php/foo/bar', 'no params using empty array'),
+			array('controller3', array('p' => 3), true, false, 'app.php/foo/bar/p-3', 'no params using empty array'),
+		);
+	}
+
+	/**
+	* @dataProvider helper_url_data_no_rewrite()
+	*/
+	public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
+	{
+		$this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, $this->provider, '', 'php');
+		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id));
+	}
+
+	public function helper_url_data_with_rewrite()
+	{
+		return array(
+			array('controller2', array('t' => 1, 'f' => 2), true, false, 'foo/bar?t=1&amp;f=2', 'parameters in params-argument as array'),
+			array('controller2', array('t' => 1, 'f' => 2), false, false, 'foo/bar?t=1&f=2', 'parameters in params-argument as array'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, false, 'foo/bar/p-3?t=1&amp;f=2', 'parameters in params-argument as array'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2), false, false, 'foo/bar/p-3?t=1&f=2', 'parameters in params-argument as array'),
+
+			// Custom sid parameter
+			array('controller2', array('t' => 1, 'f' => 2), true, 'custom-sid', 'foo/bar?t=1&amp;f=2&amp;sid=custom-sid', 'params-argument (array) using session_id'),
+			array('controller2', array('t' => 1, 'f' => 2), false, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid', 'params-argument (array) using session_id'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2), true, 'custom-sid', 'foo/bar/p-3?t=1&amp;f=2&amp;sid=custom-sid', 'params-argument (array) using session_id'),
+
+			// Testing anchors
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'foo/bar?t=1&amp;f=2#anchor', 'anchor in params-argument (array)'),
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, false, 'foo/bar?t=1&f=2#anchor', 'anchor in params-argument (array)'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'foo/bar/p-3?t=1&amp;f=2#anchor', 'anchor in params-argument (array)'),
+
+			// Anchors and custom sid
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'foo/bar?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
+			array('controller2', array('t' => 1, 'f' => 2, '#' => 'anchor'), false, 'custom-sid', 'foo/bar?t=1&f=2&sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
+			array('controller3', array('p' => 3, 't' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'foo/bar/p-3?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
+
+			// Empty parameters should not append the &amp; or ?
+			array('controller2', array(), true, false, 'foo/bar', 'no params using empty array'),
+			array('controller2', array(), false, false, 'foo/bar', 'no params using empty array'),
+			array('controller3', array('p' => 3), true, false, 'foo/bar/p-3', 'no params using empty array'),
+		);
+	}
+
+	/**
+	* @dataProvider helper_url_data_with_rewrite()
+	*/
+	public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
+	{
+		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
+		$this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, $this->provider, '', 'php');
+		$this->assertEquals($expected, $this->helper->route($route, $params, $is_amp, $session_id));
+	}
+}

tests/controller/helper_url_test.php

@@ -1,119 +0,0 @@
-<?php
-/**
-*
-* @package testing
-* @copyright (c) 2013 phpBB Group
-* @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License v2
-*
-*/
-
-require_once dirname(__FILE__) . '/../../phpBB/includes/functions.php';
-
-class phpbb_controller_helper_url_test extends phpbb_test_case
-{
-
-	public function helper_url_data_no_rewrite()
-	{
-		return array(
-			array('foo/bar?t=1&amp;f=2', false, true, false, 'app.php/foo/bar?t=1&amp;f=2', 'parameters in url-argument'),
-			array('foo/bar', 't=1&amp;f=2', true, false, 'app.php/foo/bar?t=1&amp;f=2', 'parameters in params-argument using amp'),
-			array('foo/bar', 't=1&f=2', false, false, 'app.php/foo/bar?t=1&f=2', 'parameters in params-argument using &'),
-			array('foo/bar', array('t' => 1, 'f' => 2), true, false, 'app.php/foo/bar?t=1&amp;f=2', 'parameters in params-argument as array'),
-
-			// Custom sid parameter
-			array('foo/bar', 't=1&amp;f=2', true, 'custom-sid', 'app.php/foo/bar?t=1&amp;f=2&amp;sid=custom-sid', 'using session_id'),
-
-			// Testing anchors
-			array('foo/bar?t=1&amp;f=2#anchor', false, true, false, 'app.php/foo/bar?t=1&amp;f=2#anchor', 'anchor in url-argument'),
-			array('foo/bar', 't=1&amp;f=2#anchor', true, false, 'app.php/foo/bar?t=1&amp;f=2#anchor', 'anchor in params-argument'),
-			array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'app.php/foo/bar?t=1&amp;f=2#anchor', 'anchor in params-argument (array)'),
-
-			// Anchors and custom sid
-			array('foo/bar?t=1&amp;f=2#anchor', false, true, 'custom-sid', 'app.php/foo/bar?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in url-argument using session_id'),
-			array('foo/bar', 't=1&amp;f=2#anchor', true, 'custom-sid', 'app.php/foo/bar?t=1&amp;f=2&amp;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/foo/bar?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
-
-			// Empty parameters should not append the &amp;
-			array('foo/bar', false, true, false, 'app.php/foo/bar', 'no params using bool false'),
-			array('foo/bar', '', true, false, 'app.php/foo/bar', 'no params using empty string'),
-			array('foo/bar', array(), true, false, 'app.php/foo/bar', 'no params using empty array'),
-		);
-	}
-
-	/**
-	* @dataProvider helper_url_data_no_rewrite()
-	*/
-	public function test_helper_url_no_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
-	{
-		global $phpbb_dispatcher, $phpbb_root_path, $phpEx;
-
-		$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
-		$this->user = $this->getMock('\phpbb\user');
-		$phpbb_path_helper = new \phpbb\path_helper(
-			new \phpbb\symfony_request(
-				new phpbb_mock_request()
-			),
-			new \phpbb\filesystem(),
-			$phpbb_root_path,
-			$phpEx
-		);
-		$this->template = new phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context());
-
-		// We don't use mod_rewrite in these tests
-		$config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
-		$helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php');
-		$this->assertEquals($helper->url($route, $params, $is_amp, $session_id), $expected);
-	}
-
-	public function helper_url_data_with_rewrite()
-	{
-		return array(
-			array('foo/bar?t=1&amp;f=2', false, true, false, 'foo/bar?t=1&amp;f=2', 'parameters in url-argument'),
-			array('foo/bar', 't=1&amp;f=2', true, false, 'foo/bar?t=1&amp;f=2', 'parameters in params-argument using amp'),
-			array('foo/bar', 't=1&f=2', false, false, 'foo/bar?t=1&f=2', 'parameters in params-argument using &'),
-			array('foo/bar', array('t' => 1, 'f' => 2), true, false, 'foo/bar?t=1&amp;f=2', 'parameters in params-argument as array'),
-
-			// Custom sid parameter
-			array('foo/bar', 't=1&amp;f=2', true, 'custom-sid', 'foo/bar?t=1&amp;f=2&amp;sid=custom-sid', 'using session_id'),
-
-			// Testing anchors
-			array('foo/bar?t=1&amp;f=2#anchor', false, true, false, 'foo/bar?t=1&amp;f=2#anchor', 'anchor in url-argument'),
-			array('foo/bar', 't=1&amp;f=2#anchor', true, false, 'foo/bar?t=1&amp;f=2#anchor', 'anchor in params-argument'),
-			array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, false, 'foo/bar?t=1&amp;f=2#anchor', 'anchor in params-argument (array)'),
-
-			// Anchors and custom sid
-			array('foo/bar?t=1&amp;f=2#anchor', false, true, 'custom-sid', 'foo/bar?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in url-argument using session_id'),
-			array('foo/bar', 't=1&amp;f=2#anchor', true, 'custom-sid', 'foo/bar?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in params-argument using session_id'),
-			array('foo/bar', array('t' => 1, 'f' => 2, '#' => 'anchor'), true, 'custom-sid', 'foo/bar?t=1&amp;f=2&amp;sid=custom-sid#anchor', 'anchor in params-argument (array) using session_id'),
-
-			// Empty parameters should not append the &amp;
-			array('foo/bar', false, true, false, 'foo/bar', 'no params using bool false'),
-			array('foo/bar', '', true, false, 'foo/bar', 'no params using empty string'),
-			array('foo/bar', array(), true, false, 'foo/bar', 'no params using empty array'),
-		);
-	}
-
-	/**
-	* @dataProvider helper_url_data_with_rewrite()
-	*/
-	public function test_helper_url_with_rewrite($route, $params, $is_amp, $session_id, $expected, $description)
-	{
-		global $phpbb_dispatcher, $phpbb_root_path, $phpEx;
-
-		$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
-		$this->user = $this->getMock('\phpbb\user');
-		$phpbb_path_helper = new \phpbb\path_helper(
-			new \phpbb\symfony_request(
-				new phpbb_mock_request()
-			),
-			new \phpbb\filesystem(),
-			$phpbb_root_path,
-			$phpEx
-		);
-		$this->template = new \phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context());
-
-		$config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
-		$helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php');
-		$this->assertEquals($helper->url($route, $params, $is_amp, $session_id), $expected);
-	}
-}

tests/functional/fixtures/ext/foo/bar/config/routing.yml

@@ -17,3 +17,15 @@ foo_exception_controller:
 foo_redirect_controller:
     pattern: /foo/redirect
     defaults: { _controller: foo_bar.controller:redirect }
+
+foo_index_controller:
+    pattern: /index
+    defaults: { _controller: foo_bar.controller:redirect }
+
+foo_tests_index_controller:
+    pattern: /tests/index
+    defaults: { _controller: foo_bar.controller:redirect }
+
+foo_tests_dotdot_index_controller:
+    pattern: /tests/../index
+    defaults: { _controller: foo_bar.controller:redirect }

tests/functional/fixtures/ext/foo/bar/controller/controller.php

@@ -63,40 +63,19 @@ class controller
 				'tests/index.php',
 			),
 			array(
-				$this->helper->url('index'),
+				$this->helper->route('foo_index_controller'),
 				$rewrite_prefix . 'index',
 			),
 			array(
-				$this->helper->url('tests/index'),
+				$this->helper->route('foo_tests_index_controller'),
 				$rewrite_prefix . 'tests/index',
 			),
+			/**
+			* Symfony does not allow /../ in routes
 			array(
-				$this->helper->url('tests/../index'),
+				$this->helper->route('foo_tests_dotdot_index_controller'),
 				$rewrite_prefix . 'index',
 			),
-			/*
-			// helper URLs starting with  ../ are prone to failure.
-			// Do not test them right now.
-			array(
-				$this->helper->url('../index'),
-				'../index',
-			),
-			array(
-				$this->helper->url('../../index'),
-				'../index',
-			),
-			array(
-				$this->helper->url('../tests/index'),
-				$rewrite_prefix . '../tests/index',
-			),
-			array(
-				$this->helper->url('../tests/../index'),
-				'../index',
-			),
-			array(
-				$this->helper->url('../../tests/index'),
-				'../tests/index',
-			),
 			*/
 		);
 

tests/pagination/config/routing.yml

@@ -0,0 +1,6 @@
+core_controller:
+    pattern: /test
+    defaults: { _controller: core_foo.controller:bar, page: 1}
+core_page_controller:
+    pattern: /test/page/{page}
+    defaults: { _controller: core_foo.controller:bar}

tests/pagination/pagination_test.php

@@ -21,11 +21,27 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
 	public function setUp()
 	{
 		parent::setUp();
-		$user = $this->getMock('\phpbb\user');
+
-		$user->expects($this->any())
+		global $phpbb_dispatcher;
+
+		$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
+		$this->user = $this->getMock('\phpbb\user');
+		$this->user->expects($this->any())
 			->method('lang')
 			->will($this->returnCallback(array($this, 'return_callback_implode')));
-		$this->pagination = new \phpbb\pagination($this->template, $user);
+
+		$this->finder = new \phpbb\extension\finder(
+			new phpbb_mock_extension_manager(dirname(__FILE__) . '/', array()),
+			new \phpbb\filesystem(),
+			dirname(__FILE__) . '/',
+			new phpbb_mock_cache()
+		);
+
+		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
+		$provider = new \phpbb\controller\provider($this->finder);
+		$provider->find(dirname(__FILE__) . '/');
+		$this->helper = new \phpbb\controller\helper($this->template, $this->user, $this->config, $provider, '', 'php');
+		$this->pagination = new \phpbb\pagination($this->template, $this->user, $this->helper);
 	}
 
 	public function generate_template_pagination_data()
@@ -77,15 +93,18 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
 				:u_next:page.php?start=30',
 			),
 			array(
-				'test/page/%d',
+				array('routes' => array(
-				'/page/%d',
+					'core_controller',
+					'core_page_controller',
+				)),
+				'page',
 				95,
 				10,
 				10,
 				'pagination
 				:per_page:10
 				:current_page:2
-				:base_url:test/page/%d
+				:base_url:
 				:previous::test
 				:else:1:test
 				:current:2:test/page/2
@@ -99,15 +118,18 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
 				:u_next:test/page/3',
 			),
 			array(
-				'test/page/%d',
+				array('routes' => array(
-				'/page/%d',
+					'core_controller',
+					'core_page_controller',
+				)),
+				'page',
 				95,
 				10,
 				20,
 				'pagination
 				:per_page:10
 				:current_page:3
-				:base_url:test/page/%d
+				:base_url:
 				:previous::test/page/2
 				:else:1:test
 				:else:2:test/page/2

tests/security/redirect_test.php

@@ -15,11 +15,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 {
 	protected $path_helper;
 
-	protected $controller_helper;
-
 	public function provider()
 	{
-		$this->controller_helper = $this->get_controller_helper();
 		// array(Input -> redirect(), expected triggered error (else false), expected returned result url (else false))
 		return array(
 			array('data://x', false, false, 'http://localhost/phpBB'),
@@ -38,8 +35,8 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 			array('./../foo/bar', false, false, 'http://localhost/foo/bar'),
 			array('./../foo/bar', true, false, 'http://localhost/foo/bar'),
 			array('app.php/', false, false, 'http://localhost/phpBB/app.php/'),
-			array($this->controller_helper->url('a'), false, false, 'http://localhost/phpBB/app.php/a'),
+			array('app.php/a', false, false, 'http://localhost/phpBB/app.php/a'),
-			array($this->controller_helper->url(''), false, false, 'http://localhost/phpBB/app.php/'),
+			array('app.php/a/b', false, false, 'http://localhost/phpBB/app.php/a/b'),
 			array('./app.php/', false, false, 'http://localhost/phpBB/app.php/'),
 			array('foobar', false, false, 'http://localhost/phpBB/foobar'),
 			array('./foobar', false, false, 'http://localhost/phpBB/foobar'),
@@ -69,31 +66,6 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 		return $this->path_helper;
 	}
 
-	protected function get_controller_helper()
-	{
-		if (!($this->controller_helper instanceof \phpbb\controller\helper))
-		{
-			global $phpbb_dispatcher;
-
-			$phpbb_dispatcher = new phpbb_mock_event_dispatcher;
-			$this->user = $this->getMock('\phpbb\user');
-			$phpbb_path_helper = new \phpbb\path_helper(
-				new \phpbb\symfony_request(
-					new phpbb_mock_request()
-				),
-				new \phpbb\filesystem(),
-				$phpbb_root_path,
-				$phpEx
-			);
-			$this->template = new phpbb\template\twig\twig($phpbb_path_helper, $config, $this->user, new \phpbb\template\context());
-
-			// We don't use mod_rewrite in these tests
-			$config = new \phpbb\config\config(array('enable_mod_rewrite' => '0'));
-			$this->controller_helper = new \phpbb\controller\helper($this->template, $this->user, $config, '', 'php');
-		}
-		return $this->controller_helper;
-	}
-
 	protected function setUp()
 	{
 		parent::setUp();
@@ -103,7 +75,6 @@ class phpbb_security_redirect_test extends phpbb_security_test_base
 		);
 
 		$this->path_helper = $this->get_path_helper();
-		$this->controller_helper = $this->get_controller_helper();
 	}
 
 	/**