Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FrameworkBundle][Routing] Add a new tag to be able to use a private service as a service route loader #30926

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions UPGRADE-4.3.md
Expand Up @@ -123,6 +123,7 @@ Routing
options have been deprecated.
* Implementing `Serializable` for `Route` and `CompiledRoute` is deprecated; if you serialize them, please
ensure your unserialization logic can recover from a failure related to an updated serialization format
* Not tagging the router loader services with `routing.router_loader` has been deprecated.

Security
--------
Expand Down
1 change: 1 addition & 0 deletions UPGRADE-5.0.md
Expand Up @@ -284,6 +284,7 @@ Routing
options have been removed.
* `Route` and `CompiledRoute` don't implement `Serializable` anymore; if you serialize them, please
ensure your unserialization logic can recover from a failure related to an updated serialization format
* The router loader services must be tagged with `routing.router_loader`.

Security
--------
Expand Down
Expand Up @@ -49,6 +49,7 @@ class UnusedTagsPass implements CompilerPassInterface
'proxy',
'routing.expression_language_provider',
'routing.loader',
'routing.router_loader',
'security.expression_language_provider',
'security.remember_me_aware',
'security.voter',
Expand Down
Expand Up @@ -92,6 +92,7 @@
use Symfony\Component\Routing\Generator\UrlGenerator;
use Symfony\Component\Routing\Loader\AnnotationDirectoryLoader;
use Symfony\Component\Routing\Loader\AnnotationFileLoader;
use Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoaderInterface;
use Symfony\Component\Routing\Matcher\CompiledUrlMatcher;
use Symfony\Component\Routing\Matcher\Dumper\PhpMatcherDumper;
use Symfony\Component\Security\Core\Security;
Expand Down Expand Up @@ -428,6 +429,9 @@ public function load(array $configs, ContainerBuilder $container)
if (!$config['disallow_search_engine_index'] ?? false) {
$container->removeDefinition('disallow_search_engine_index_response_listener');
}

$container->registerForAutoconfiguration(ServiceRouterLoaderInterface::class)
->addTag('routing.router_loader');
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
13 changes: 11 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php
Expand Up @@ -69,14 +69,23 @@ public function registerContainerConfiguration(LoaderInterface $loader)
],
]);

if ($this instanceof EventSubscriberInterface) {
if (!$container->hasDefinition('kernel')) {
$container->register('kernel', static::class)
->setSynthetic(true)
->setPublic(true)
->addTag('kernel.event_subscriber')
;
}

$kernelDefinition = $container->getDefinition('kernel');

$kernelDefinition->addTag('routing.router_loader', [
'service_id' => 'kernel',
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
]);

if ($this instanceof EventSubscriberInterface) {
$kernelDefinition->addTag('kernel.event_subscriber');
}

$this->configureContainer($container, $loader);

$container->addObjectResource($this);
Expand Down
Expand Up @@ -40,9 +40,14 @@
<argument type="service" id="file_locator" />
</service>

<service id="routing.loader.service.container" class="Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoaderContainer">
<argument type="service" id="service_container" />
<argument type="tagged_locator" tag="routing.router_loader" index-by="service_id" />
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
</service>

<service id="routing.loader.service" class="Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoader">
<tag name="routing.loader" />
<argument type="service" id="service_container" />
<argument type="service" id="routing.loader.service.container" />
Copy link
Contributor Author

@fancyweb fancyweb Apr 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 5.0, replace this argument with the above tagged locator.

</service>

<service id="routing.loader" class="Symfony\Bundle\FrameworkBundle\Routing\DelegatingLoader" public="true">
Expand Down
Expand Up @@ -12,6 +12,9 @@
namespace Symfony\Bundle\FrameworkBundle\Tests\Kernel;

use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Extension\ExtensionInterface;
use Symfony\Component\DependencyInjection\Loader\ClosureLoader;
use Symfony\Component\HttpFoundation\Request;

class MicroKernelTraitTest extends TestCase
Expand Down Expand Up @@ -39,4 +42,21 @@ public function testAsEventSubscriber()

$this->assertSame('It\'s dangerous to go alone. Take this ⚔', $response->getContent());
}

public function testRoutingRouteLoaderTagIsAdded()
{
$frameworkExtension = $this->createMock(ExtensionInterface::class);
$frameworkExtension
->expects($this->atLeastOnce())
->method('getAlias')
->willReturn('framework');

$container = new ContainerBuilder();
$container->registerExtension($frameworkExtension);

$kernel = new ConcreteMicroKernel('test', false);
$kernel->registerContainerConfiguration(new ClosureLoader($container));

$this->assertTrue($container->getDefinition('kernel')->hasTag('routing.router_loader'));
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Component/Routing/CHANGELOG.md
Expand Up @@ -11,7 +11,7 @@ CHANGELOG
* deprecated implementing `Serializable` for `Route` and `CompiledRoute`; if you serialize them, please
ensure your unserialization logic can recover from a failure related to an updated serialization format
* exposed `utf8` Route option, defaults "locale" and "format" in configuration loaders and configurators
* added support for invokable route loader services
* added support for invokable router loader services

4.2.0
-----
Expand Down
@@ -0,0 +1,53 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Routing\Loader\DependencyInjection;

use Psr\Container\ContainerInterface;

/**
* @internal
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
*
* @deprecated since Symfony 4.3, to be removed in 5.0
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
*/
class ServiceRouterLoaderContainer implements ContainerInterface
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RouterLoaderContainer?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
private $container;
private $serviceLocator;

public function __construct(ContainerInterface $container, ContainerInterface $serviceLocator)
{
$this->container = $container;
$this->serviceLocator = $serviceLocator;
}

/**
* {@inheritdoc}
*/
public function get($id)
{
if ($this->serviceLocator->has($id)) {
return $this->serviceLocator->get($id);
}

@trigger_error(sprintf('Registering the routing loader "%s" without tagging it with the "routing.router_loader" tag is deprecated since Symfony 4.3 and will be required in Symfony 5.0.', $id), E_USER_DEPRECATED);

return $this->container->get($id);
}

/**
* {@inheritdoc}
*/
public function has($id)
{
return $this->serviceLocator->has($id) || $this->container->has($id);
}
}
@@ -0,0 +1,19 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Routing\Loader\DependencyInjection;

/**
* Marker interface for router loader services.
*/
interface ServiceRouterLoaderInterface
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
fancyweb marked this conversation as resolved.
Show resolved Hide resolved
{
}
@@ -0,0 +1,66 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Component\Routing\Tests\Loader;

use PHPUnit\Framework\TestCase;
use Psr\Container\ContainerInterface;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoaderContainer;

class ServiceRouterLoaderContainerTest extends TestCase
{
/**
* @var ContainerInterface
*/
private $container;

/**
* @var ContainerInterface
*/
private $serviceLocator;

/**
* @var ServiceRouterLoaderContainer
*/
private $serviceRouterLoaderContainer;

/**
* {@inheritdoc}
*/
protected function setUp()
{
$this->container = new Container();
$this->container->set('foo', new \stdClass());

$this->serviceLocator = new Container();
$this->serviceLocator->set('bar', new \stdClass());

$this->serviceRouterLoaderContainer = new ServiceRouterLoaderContainer($this->container, $this->serviceLocator);
}

/**
* @group legacy
* @expectedDeprecation Registering the routing loader "foo" without tagging it with the "routing.router_loader" tag is deprecated since Symfony 4.3 and will be required in Symfony 5.0.
*/
public function testGet()
{
$this->assertSame($this->container->get('foo'), $this->serviceRouterLoaderContainer->get('foo'));
$this->assertSame($this->serviceLocator->get('bar'), $this->serviceRouterLoaderContainer->get('bar'));
}

public function testHas()
{
$this->assertTrue($this->serviceRouterLoaderContainer->has('foo'));
$this->assertTrue($this->serviceRouterLoaderContainer->has('bar'));
$this->assertFalse($this->serviceRouterLoaderContainer->has('ccc'));
}
}