Skip to content

Commit

Permalink
feature #24738 [FrameworkBundle][Routing] Use a PSR-11 container in F…
Browse files Browse the repository at this point in the history
…rameworkBundle Router (ogizanagi)

This PR was merged into the 4.1-dev branch.

Discussion
----------

[FrameworkBundle][Routing] Use a PSR-11 container in FrameworkBundle Router

| Q             | A
| ------------- | ---
| Branch?       | 4.1 <!-- see comment below -->
| Bug fix?      | no
| New feature?  | yes <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no
| Deprecations? | not yet <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets | N/A <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | N/A

~3.4 because~ it allows to make the `routing.loader` service private and add sense into implementing the `ServiceSubscriberInterface` in the `Router` by injecting a ServiceLocator instead of the DI container.

Should we deprecate passing a DI `ContainerInterface` instance without providing the `$paramFetcher` argument?
Move the whole `Router::resolve()` method into a dedicated `callable $paramResolver` ?

Commits
-------

5a2f295 [FrameworkBundle][Routing] Use a PSR-11 container & parameter bag in FrameworkBundle Router
  • Loading branch information
fabpot committed Dec 11, 2017
2 parents 2677109 + 5a2f295 commit 74106c2
Show file tree
Hide file tree
Showing 5 changed files with 282 additions and 14 deletions.
1 change: 1 addition & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@ CHANGELOG

* Allowed to pass an optional `LoggerInterface $logger` instance to the `Router`
* Added a new `parameter_bag` service with related autowiring aliases to access parameters as-a-service
* Allowed the `Router` to work with any PSR-11 container

4.0.0
-----
Expand Down
Expand Up @@ -619,6 +619,13 @@ private function registerRouterConfiguration(array $config, ContainerBuilder $co

$loader->load('routing.xml');

if (!interface_exists(ContainerBagInterface::class)) {
$container->getDefinition('router.default')
->replaceArgument(0, new Reference('service_container'))
->clearTag('container.service_subscriber')
;
}

$container->setParameter('router.resource', $config['resource']);
$container->setParameter('router.cache_class_prefix', $container->getParameter('kernel.container_class'));
$router = $container->findDefinition('router.default');
Expand Down
Expand Up @@ -52,7 +52,8 @@

<service id="router.default" class="Symfony\Bundle\FrameworkBundle\Routing\Router">
<tag name="monolog.logger" channel="router" />
<argument type="service" id="service_container" />
<tag name="container.service_subscriber" id="routing.loader" />
<argument type="service" id="Psr\Container\ContainerInterface" />
<argument>%router.resource%</argument>
<argument type="collection">
<argument key="cache_dir">%kernel.cache_dir%</argument>
Expand All @@ -67,6 +68,7 @@
<argument key="matcher_cache_class">%router.cache_class_prefix%UrlMatcher</argument>
</argument>
<argument type="service" id="router.request_context" on-invalid="ignore" />
<argument type="service" id="parameter_bag" on-invalid="ignore" />
<argument type="service" id="logger" on-invalid="ignore" />
<call method="setConfigCacheFactory">
<argument type="service" id="config_cache_factory" />
Expand Down
32 changes: 20 additions & 12 deletions src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
Expand Up @@ -11,13 +11,14 @@

namespace Symfony\Bundle\FrameworkBundle\Routing;

use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\Config\ContainerParametersResource;
use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface;
use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
use Symfony\Component\Routing\Router as BaseRouter;
use Symfony\Component\Routing\RequestContext;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface;
use Symfony\Component\DependencyInjection\Exception\ParameterNotFoundException;
Expand All @@ -32,22 +33,31 @@ class Router extends BaseRouter implements WarmableInterface, ServiceSubscriberI
{
private $container;
private $collectedParameters = array();
private $paramFetcher;

/**
* @param ContainerInterface $container A ContainerInterface instance
* @param mixed $resource The main resource to load
* @param array $options An array of options
* @param RequestContext $context The context
* @param LoggerInterface|null $logger
* @param ContainerInterface $container A ContainerInterface instance
* @param mixed $resource The main resource to load
* @param array $options An array of options
* @param RequestContext $context The context
* @param ContainerInterface|null $parameters A ContainerInterface instance allowing to fetch parameters
* @param LoggerInterface|null $logger
*/
public function __construct(ContainerInterface $container, $resource, array $options = array(), RequestContext $context = null, LoggerInterface $logger = null)
public function __construct(ContainerInterface $container, $resource, array $options = array(), RequestContext $context = null, ContainerInterface $parameters = null, LoggerInterface $logger = null)
{
$this->container = $container;

$this->resource = $resource;
$this->context = $context ?: new RequestContext();
$this->logger = $logger;
$this->setOptions($options);

if ($parameters) {
$this->paramFetcher = array($parameters, 'get');
} elseif ($container instanceof SymfonyContainerInterface) {
$this->paramFetcher = array($container, 'getParameter');
} else {
throw new \LogicException(sprintf('You should either pass a "%s" instance or provide the $parameters argument of the "%s" method.', SymfonyContainerInterface::class, __METHOD__));
}
}

/**
Expand Down Expand Up @@ -142,9 +152,7 @@ private function resolve($value)
return $value;
}

$container = $this->container;

$escapedValue = preg_replace_callback('/%%|%([^%\s]++)%/', function ($match) use ($container, $value) {
$escapedValue = preg_replace_callback('/%%|%([^%\s]++)%/', function ($match) use ($value) {
// skip %%
if (!isset($match[1])) {
return '%%';
Expand All @@ -154,7 +162,7 @@ private function resolve($value)
throw new RuntimeException(sprintf('Using "%%%s%%" is not allowed in routing configuration.', $match[1]));
}

$resolved = $container->getParameter($match[1]);
$resolved = ($this->paramFetcher)($match[1]);

if (is_string($resolved) || is_numeric($resolved)) {
$this->collectedParameters[$match[1]] = $resolved;
Expand Down

0 comments on commit 74106c2

Please sign in to comment.