[ticket/13388] Integrate routing and di parameters resolution

PHPBB3-13388
author: Tristan Darricau <tristan.darricau@sensiolabs.com> 2015-05-04 15:10:33 +0200
committer: Tristan Darricau <tristan.darricau@sensiolabs.com> 2015-05-29 16:48:51 +0200
commit: 66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11 (patch)
tree: 34b18d59a27f7c870c7d58dee30096e42743d06a
parent: 86ad1a92c87a6bde184b59a981b40752bf2f61b1 (diff)
download: forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar
forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar.gz
forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar.bz2
forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar.xz
forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.zip
6 files changed, 156 insertions, 10 deletions
diff --git a/phpBB/config/default/container/services_routing.yml b/phpBB/config/default/container/services_routing.yml
index 6749202c0d..f76b5e5ede 100644
--- a/phpBB/config/default/container/services_routing.yml
+++ b/phpBB/config/default/container/services_routing.yml
@@ -2,6 +2,7 @@ services:
     router:
         class: phpbb\routing\router
         arguments:
+            - @service_container
             - @filesystem
             - %core.root_path%
             - %core.php_ext%
diff --git a/phpBB/phpbb/routing/router.php b/phpBB/phpbb/routing/router.php
index 2f89d4e884..ca9799ba3e 100644
--- a/phpBB/phpbb/routing/router.php
+++ b/phpBB/phpbb/routing/router.php
@@ -15,6 +15,9 @@ namespace phpbb\routing;
 
 use Symfony\Component\Config\ConfigCache;
 use Symfony\Component\Filesystem\Exception\IOException;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper;
 use Symfony\Component\Routing\Generator\Dumper\PhpGeneratorDumper;
 use Symfony\Component\Routing\Matcher\UrlMatcher;
@@ -92,17 +95,22 @@ class router implements RouterInterface
 	protected $filesystem;
 
 	/**
+	 * @var ContainerInterface
+	 */
+	protected $container;
+
+	/**
 	 * Construct method
 	 *
+	 * @param ContainerInterface	$container			DI container
 	 * @param \phpbb\filesystem\filesystem_interface $filesystem	Filesystem helper
-	 * @param string		$phpbb_root_path						phpBB root path
-	 * @param string		$php_ext								PHP file extension
-	 * @param string		$environment							Name of the current environment
-	 * @param manager|null	$extension_manager						Extension manager
-	 * @param array			$routing_files							Array of strings containing paths to YAML files
-	 * 																holding route information
+	 * @param string	$phpbb_root_path	phpBB root path
+	 * @param string	$php_ext			PHP file extension
+	 * @param string	$environment		Name of the current environment
+	 * @param manager	$extension_manager	Extension manager
+	 * @param array		$routing_files		Array of strings containing paths to YAML files holding route information
 	 */
-	public function __construct(\phpbb\filesystem\filesystem_interface $filesystem, $phpbb_root_path, $php_ext, $environment, manager $extension_manager = null, $routing_files = array())
+	public function __construct(ContainerInterface $container, \phpbb\filesystem\filesystem_interface $filesystem, $phpbb_root_path, $php_ext, $environment, manager $extension_manager = null, $routing_files = array())
 	{
 		$this->filesystem			= $filesystem;
 		$this->extension_manager	= $extension_manager;
@@ -162,10 +170,118 @@ class router implements RouterInterface
 			}
 		}
 
+		$this->resolveParameters($this->route_collection);
+
 		return $this;
 	}
 
 	/**
+	 * Replaces placeholders with service container parameter values in:
+	 * - the route defaults,
+	 * - the route requirements,
+	 * - the route path,
+	 * - the route host,
+	 * - the route schemes,
+	 * - the route methods.
+	 *
+	 * @param RouteCollection $collection
+	 */
+	private function resolveParameters(RouteCollection $collection)
+	{
+		foreach ($collection as $route)
+		{
+			foreach ($route->getDefaults() as $name => $value)
+			{
+				$route->setDefault($name, $this->resolve($value));
+			}
+
+			foreach ($route->getRequirements() as $name => $value)
+			{
+				if ($name === '_scheme' || $name === '_method')
+				{
+					continue; // ignore deprecated requirements to not trigger deprecation warnings
+				}
+
+				$route->setRequirement($name, $this->resolve($value));
+			}
+
+			$route->setPath($this->resolve($route->getPath()));
+			$route->setHost($this->resolve($route->getHost()));
+
+			$schemes = array();
+			foreach ($route->getSchemes() as $scheme)
+			{
+				$schemes = array_merge($schemes, explode('|', $this->resolve($scheme)));
+			}
+
+			$route->setSchemes($schemes);
+			$methods = array();
+			foreach ($route->getMethods() as $method)
+			{
+				$methods = array_merge($methods, explode('|', $this->resolve($method)));
+			}
+
+			$route->setMethods($methods);
+			$route->setCondition($this->resolve($route->getCondition()));
+		}
+	}
+	/**
+	 * Recursively replaces placeholders with the service container parameters.
+	 *
+	 * @param mixed $value The source which might contain "%placeholders%"
+	 *
+	 * @return mixed The source with the placeholders replaced by the container
+	 *               parameters. Arrays are resolved recursively.
+	 *
+	 * @throws ParameterNotFoundException When a placeholder does not exist as a container parameter
+	 * @throws RuntimeException           When a container value is not a string or a numeric value
+	 */
+	private function resolve($value)
+	{
+		if (is_array($value))
+		{
+			foreach ($value as $key => $val)
+			{
+				$value[$key] = $this->resolve($val);
+			}
+
+			return $value;
+		}
+
+		if (!is_string($value))
+		{
+			return $value;
+		}
+
+		$container = $this->container;
+		$escapedValue = preg_replace_callback('/%%|%([^%\s]++)%/', function ($match) use ($container, $value)
+		{
+			// skip %%
+			if (!isset($match[1]))
+			{
+				return '%%';
+			}
+
+			$resolved = $container->getParameter($match[1]);
+			if (is_string($resolved) || is_numeric($resolved))
+			{
+				return (string) $resolved;
+			}
+
+			throw new RuntimeException(sprintf(
+					'The container parameter "%s", used in the route configuration value "%s", '.
+					'must be a string or numeric, but it is of type %s.',
+					$match[1],
+					$value,
+					gettype($resolved)
+				)
+			);
+		}, $value);
+
+		return str_replace('%%', '%', $escapedValue);
+	}
+
+	/**
 	 * Get the list of routes
 	 *
 	 * @return RouteCollection Get the route collection
diff --git a/tests/controller/common_helper_route.php b/tests/controller/common_helper_route.php
index 44e5e12ab0..3c74c16bae 100644
--- a/tests/controller/common_helper_route.php
+++ b/tests/controller/common_helper_route.php
@@ -89,6 +89,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 		$this->user = new \phpbb\user($lang, '\phpbb\datetime');;
 
 		$container = new phpbb_mock_container_builder();
+		$container->setParameter('core.environment', PHPBB_ENVIRONMENT);
 		$cache_path = $phpbb_root_path . 'cache/twig';
 		$context = new \phpbb\template\context();
 		$loader = new \phpbb\template\twig\loader($this->filesystem, '');
@@ -121,7 +122,7 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			)
 		);
 
-		$this->router = new phpbb_mock_router($this->filesystem, dirname(__FILE__) . '/', 'php', PHPBB_ENVIRONMENT, $this->extension_manager);
+		$this->router = new phpbb_mock_router($container, $this->filesystem, dirname(__FILE__) . '/', 'php', PHPBB_ENVIRONMENT, $this->extension_manager);
 		$this->router->find_routing_files($this->extension_manager->all_enabled(false));
 		$this->router->find(dirname(__FILE__) . '/');
 		// Set correct current phpBB root path
@@ -155,6 +156,9 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			array('controller2', array(), true, false, '/' . $this->path_to_app() . 'app.php/foo/bar', 'no params using empty array'),
 			array('controller2', array(), false, false, '/' . $this->path_to_app() . 'app.php/foo/bar', 'no params using empty array'),
 			array('controller3', array('p' => 3), true, false, '/' . $this->path_to_app() . 'app.php/foo/bar/p-3', 'no params using empty array'),
+
+			// Resolves DI parameters
+			array('controller4', array(), true, false, '/' . $this->path_to_app() . 'app.php/foo/' . PHPBB_ENVIRONMENT, 'di parameter'),
 		);
 	}
 
@@ -195,6 +199,9 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			array('controller2', array(), true, false, '/' . $this->path_to_app() . 'foo/bar', 'no params using empty array'),
 			array('controller2', array(), false, false, '/' . $this->path_to_app() . 'foo/bar', 'no params using empty array'),
 			array('controller3', array('p' => 3), true, false, '/' . $this->path_to_app() . 'foo/bar/p-3', 'no params using empty array'),
+
+			// Resolves DI parameters
+			array('controller4', array(), true, false, '/' . $this->path_to_app() . 'foo/' . PHPBB_ENVIRONMENT, 'di parameter'),
 		);
 	}
 
@@ -235,6 +242,9 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			array('controller2', array(), true, false, 'http://localhost/' . $this->path_to_app() . 'app.php/foo/bar', 'no params using empty array'),
 			array('controller2', array(), false, false, 'http://localhost/' . $this->path_to_app() . 'app.php/foo/bar', 'no params using empty array'),
 			array('controller3', array('p' => 3), true, false, 'http://localhost/' . $this->path_to_app() . 'app.php/foo/bar/p-3', 'no params using empty array'),
+
+			// Resolves DI parameters
+			array('controller4', array(), true, false, 'http://localhost/' . $this->path_to_app() . 'app.php/foo/' . PHPBB_ENVIRONMENT, 'di parameter'),
 		);
 	}
 
@@ -275,6 +285,9 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			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'),
+
+			// Resolves DI parameters
+			array('controller4', array(), true, false,  'app.php/foo/' . PHPBB_ENVIRONMENT, 'di parameter'),
 		);
 	}
 
@@ -315,6 +328,9 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			array('controller2', array(), true, false, '//localhost/' . $this->path_to_app() . 'app.php/foo/bar', 'no params using empty array'),
 			array('controller2', array(), false, false, '//localhost/' . $this->path_to_app() . 'app.php/foo/bar', 'no params using empty array'),
 			array('controller3', array('p' => 3), true, false, '//localhost/' . $this->path_to_app() . 'app.php/foo/bar/p-3', 'no params using empty array'),
+
+			// Resolves DI parameters
+			array('controller4', array(), true, false, '//localhost/' . $this->path_to_app() . 'app.php/foo/' . PHPBB_ENVIRONMENT, 'di parameter'),
 		);
 	}
 
@@ -355,6 +371,9 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			array('controller2', array(), true, false, 'http://localhost/' . $this->path_to_app() . 'foo/bar', 'no params using empty array'),
 			array('controller2', array(), false, false, 'http://localhost/' . $this->path_to_app() . 'foo/bar', 'no params using empty array'),
 			array('controller3', array('p' => 3), true, false, 'http://localhost/' . $this->path_to_app() . 'foo/bar/p-3', 'no params using empty array'),
+
+			// Resolves DI parameters
+			array('controller4', array(), true, false, 'http://localhost/' . $this->path_to_app() . 'foo/' . PHPBB_ENVIRONMENT, 'di parameter'),
 		);
 	}
 
@@ -435,6 +454,9 @@ abstract class phpbb_controller_common_helper_route extends phpbb_test_case
 			array('controller2', array(), true, false, '//localhost/' . $this->path_to_app() . 'foo/bar', 'no params using empty array'),
 			array('controller2', array(), false, false, '//localhost/' . $this->path_to_app() . 'foo/bar', 'no params using empty array'),
 			array('controller3', array('p' => 3), true, false, '//localhost/' . $this->path_to_app() . 'foo/bar/p-3', 'no params using empty array'),
+
+			// Resolves DI parameters
+			array('controller4', array(), true, false, '//localhost/' . $this->path_to_app() . 'foo/' . PHPBB_ENVIRONMENT, 'di parameter'),
 		);
 	}
 
diff --git a/tests/controller/controller_test.php b/tests/controller/controller_test.php
index 5781d3ebc1..d0295d66bc 100644
--- a/tests/controller/controller_test.php
+++ b/tests/controller/controller_test.php
@@ -40,7 +40,10 @@ class phpbb_controller_controller_test extends phpbb_test_case
 
 	public function test_router_find_files()
 	{
-		$router = new \phpbb\routing\router(new \phpbb\filesystem\filesystem(), dirname(__FILE__) . '/', 'php', PHPBB_ENVIRONMENT, $this->extension_manager);
+		$container = new phpbb_mock_container_builder();
+		$container->setParameter('core.environment', PHPBB_ENVIRONMENT);
+
+		$router = new \phpbb\routing\router($container, new \phpbb\filesystem\filesystem(), dirname(__FILE__) . '/', 'php', PHPBB_ENVIRONMENT, $this->extension_manager);
 		$router->find_routing_files($this->extension_manager->all_enabled(false));
 		$routes = $router->find(__DIR__)->get_routes();
 
diff --git a/tests/controller/ext/vendor2/foo/config/routing.yml b/tests/controller/ext/vendor2/foo/config/routing.yml
index e3e8ee5f98..7d4ac7be93 100644
--- a/tests/controller/ext/vendor2/foo/config/routing.yml
+++ b/tests/controller/ext/vendor2/foo/config/routing.yml
@@ -5,3 +5,7 @@ controller1:
 include_controller2:
     resource: "routing_2.yml"
     prefix:   /foo
+
+controller4:
+    path: /foo/%core.environment%
+    defaults: { _controller: foo.controller:handle }
diff --git a/tests/pagination/pagination_test.php b/tests/pagination/pagination_test.php
index 67c3d0a30f..07f9ec9c56 100644
--- a/tests/pagination/pagination_test.php
+++ b/tests/pagination/pagination_test.php
@@ -41,7 +41,7 @@ class phpbb_pagination_pagination_test extends phpbb_template_template_test_case
 		$manager = new phpbb_mock_extension_manager(dirname(__FILE__) . '/', array());
 
 		$this->config = new \phpbb\config\config(array('enable_mod_rewrite' => '1'));
-		$router = new phpbb_mock_router($filesystem, dirname(__FILE__) . '/', 'php', PHPBB_ENVIRONMENT, $manager);
+		$router = new phpbb_mock_router(new phpbb_mock_container_builder(), $filesystem, dirname(__FILE__) . '/', 'php', PHPBB_ENVIRONMENT, $manager);
 		$router->find_routing_files($manager->all_enabled(false));
 		$router->find(dirname(__FILE__) . '/');
author	Tristan Darricau <tristan.darricau@sensiolabs.com>	2015-05-04 15:10:33 +0200
committer	Tristan Darricau <tristan.darricau@sensiolabs.com>	2015-05-29 16:48:51 +0200
commit	66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11 (patch)
tree	34b18d59a27f7c870c7d58dee30096e42743d06a
parent	86ad1a92c87a6bde184b59a981b40752bf2f61b1 (diff)
download	forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar.gz forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar.bz2 forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.tar.xz forums-66c0e0c6a83fbf091aa078ab06dd6467c8c6aa11.zip