Skip to content

Commit

Permalink
feature #34873 [FrameworkBundle] Allow using a ContainerConfigurator …
Browse files Browse the repository at this point in the history
…in MicroKernelTrait::configureContainer() (nicolas-grekas)

This PR was merged into the 5.1-dev branch.

Discussion
----------

[FrameworkBundle] Allow using a ContainerConfigurator in MicroKernelTrait::configureContainer()

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Tickets       | -
| License       | MIT
| Doc PR        | -

This PR is a continuation of #32937 (it reverts some parts of it that are not needed anymore). It builds on #34872 for now.

This PR allows using the PHP-DSL natively in our `Kernel::configureContainer()` methods.
It allows the same in our `Kernel::configureRoutes()` methods.

Both signatures are handled gracefully with no deprecations to let existing code in peace:
- `protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader);`
- `protected function configureContainer(ContainerConfigurator $c);` (a loader is always passed as 2nd arg to ease FC)

Same for routes:
- `protected function configureRoutes(RoutingConfigurator $routes);`
- `protected function configureRoutes(RouteCollectionBuilder $routes);` (this one is deprecated because `RouteCollectionBuilder` is deprecated)

Here is my working `src/Kernel.php` after this change:

```php
class Kernel extends BaseKernel
{
    use MicroKernelTrait;

    protected function configureContainer(ContainerConfigurator $container): void
    {
        $container->import('../config/{packages}/*.yaml');
        $container->import('../config/{packages}/'.$this->environment.'/*.yaml');
        $container->import('../config/services.yaml');
        $container->import('../config/{services}_'.$this->environment.'.yaml');
    }

    protected function configureRoutes(RoutingConfigurator $routes): void
    {
        $routes->import('../config/{routes}/'.$this->environment.'/*.yaml');
        $routes->import('../config/{routes}/*.yaml');
        $routes->import('../config/routes.yaml');
    }
}
```

Commits
-------

cf45eec [FrameworkBundle] Allow using a ContainerConfigurator in MicroKernelTrait::configureContainer()
  • Loading branch information
nicolas-grekas committed Dec 13, 2019
2 parents 4445812 + cf45eec commit 8c80c5b
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 161 deletions.
3 changes: 1 addition & 2 deletions UPGRADE-5.1.md
Expand Up @@ -4,8 +4,7 @@ UPGRADE FROM 5.0 to 5.1
FrameworkBundle
---------------

* Marked `MicroKernelTrait::configureRoutes()` as `@internal` and `@final`.
* Deprecated not overriding `MicroKernelTrait::configureRouting()`.
* Deprecated passing a `RouteCollectionBuiler` to `MicroKernelTrait::configureRoutes()`, type-hint `RoutingConfigurator` instead

HttpFoundation
--------------
Expand Down
3 changes: 1 addition & 2 deletions UPGRADE-6.0.md
Expand Up @@ -4,8 +4,7 @@ UPGRADE FROM 5.x to 6.0
FrameworkBundle
---------------

* Removed `MicroKernelTrait::configureRoutes()`.
* Made `MicroKernelTrait::configureRouting()` abstract.
* `MicroKernelTrait::configureRoutes()` is now always called with a `RoutingConfigurator`

HttpFoundation
--------------
Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
Expand Up @@ -4,10 +4,10 @@ CHANGELOG
5.1.0
-----

* Marked `MicroKernelTrait::configureRoutes()` as `@internal` and `@final`.
* Deprecated not overriding `MicroKernelTrait::configureRouting()`.
* Made `MicroKernelTrait::configureContainer()` compatible with `ContainerConfigurator`
* Added a new `mailer.message_bus` option to configure or disable the message bus to use to send mails.
* Added flex-compatible default implementations for `MicroKernelTrait::registerBundles()` and `getProjectDir()`
* Deprecated passing a `RouteCollectionBuiler` to `MicroKernelTrait::configureRoutes()`, type-hint `RoutingConfigurator` instead

5.0.0
-----
Expand Down
78 changes: 47 additions & 31 deletions src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php
Expand Up @@ -13,8 +13,10 @@

use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCollectionBuilder;

/**
Expand All @@ -25,20 +27,6 @@
*/
trait MicroKernelTrait
{
/**
* Add or import routes into your application.
*
* $routes->import('config/routing.yml');
* $routes->add('/admin', 'App\Controller\AdminController::dashboard', 'admin_dashboard');
*
* @final since Symfony 5.1, override configureRouting() instead
*
* @internal since Symfony 5.1, use configureRouting() instead
*/
protected function configureRoutes(RouteCollectionBuilder $routes)
{
}

/**
* Adds or imports routes into your application.
*
Expand All @@ -48,29 +36,26 @@ protected function configureRoutes(RouteCollectionBuilder $routes)
* ->controller('App\Controller\AdminController::dashboard')
* ;
*/
protected function configureRouting(RoutingConfigurator $routes): void
{
@trigger_error(sprintf('Not overriding the "%s()" method is deprecated since Symfony 5.1 and will trigger a fatal error in 6.0.', __METHOD__), E_USER_DEPRECATED);
}
abstract protected function configureRoutes(RoutingConfigurator $routes);

/**
* Configures the container.
*
* You can register extensions:
*
* $c->loadFromExtension('framework', [
* $c->extension('framework', [
* 'secret' => '%secret%'
* ]);
*
* Or services:
*
* $c->register('halloween', 'FooBundle\HalloweenProvider');
* $c->services()->set('halloween', 'FooBundle\HalloweenProvider');
*
* Or parameters:
*
* $c->setParameter('halloween', 'lot of fun');
* $c->parameters()->set('halloween', 'lot of fun');
*/
abstract protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader);
abstract protected function configureContainer(ContainerConfigurator $c);

/**
* {@inheritdoc}
Expand Down Expand Up @@ -120,9 +105,31 @@ public function registerContainerConfiguration(LoaderInterface $loader)
$kernelDefinition->addTag('kernel.event_subscriber');
}

$this->configureContainer($container, $loader);
$container->addObjectResource($this);
$container->fileExists($this->getProjectDir().'/config/bundles.php');

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

return;
} catch (\TypeError $e) {
$file = $e->getFile();

if (0 !== strpos($e->getMessage(), sprintf('Argument 1 passed to %s::configureContainer() must be an instance of %s,', static::class, ContainerConfigurator::class))) {
throw $e;
}
}

$kernelLoader = $loader->getResolver()->resolve($file);
$kernelLoader->setCurrentDir(\dirname($file));
$instanceof = &\Closure::bind(function &() { return $this->instanceof; }, $kernelLoader, $kernelLoader)();

try {
$this->configureContainer(new ContainerConfigurator($container, $kernelLoader, $instanceof, $file, $file), $loader);
} finally {
$instanceof = [];
$kernelLoader->registerAliasesForSinglyImplementedInterfaces();
}
});
}

Expand All @@ -131,17 +138,26 @@ public function registerContainerConfiguration(LoaderInterface $loader)
*/
public function loadRoutes(LoaderInterface $loader)
{
$routes = new RouteCollectionBuilder($loader);
$this->configureRoutes($routes);
$collection = $routes->build();
$file = (new \ReflectionObject($this))->getFileName();
$kernelLoader = $loader->getResolver()->resolve($file);
$kernelLoader->setCurrentDir(\dirname($file));
$collection = new RouteCollection();

if (0 !== \count($collection)) {
@trigger_error(sprintf('Adding routes via the "%s:configureRoutes()" method is deprecated since Symfony 5.1 and will have no effect in 6.0; use "configureRouting()" instead.', self::class), E_USER_DEPRECATED);
try {
$this->configureRoutes(new RoutingConfigurator($collection, $kernelLoader, $file, $file));

return $collection;
} catch (\TypeError $e) {
if (0 !== strpos($e->getMessage(), sprintf('Argument 1 passed to %s::configureRoutes() must be an instance of %s,', static::class, RouteCollectionBuilder::class))) {
throw $e;
}
}

$file = (new \ReflectionObject($this))->getFileName();
$this->configureRouting(new RoutingConfigurator($collection, $loader, null, $file));
@trigger_error(sprintf('Using type "%s" for argument 1 of method "%s:configureRoutes()" is deprecated since Symfony 5.1, use "%s" instead.', RouteCollectionBuilder::class, self::class, RoutingConfigurator::class), E_USER_DEPRECATED);

$routes = new RouteCollectionBuilder($loader);
$this->configureRoutes($routes);

return $collection;
return $routes->build();
}
}
Expand Up @@ -80,7 +80,7 @@ public function __destruct()
$fs->remove($this->cacheDir);
}

protected function configureRouting(RoutingConfigurator $routes): void
protected function configureRoutes(RoutingConfigurator $routes): void
{
$routes->add('halloween', '/')->controller('kernel::halloweenAction');
$routes->add('danger', '/danger')->controller('kernel::dangerousAction');
Expand Down
Expand Up @@ -19,18 +19,6 @@

class MicroKernelTraitTest extends TestCase
{
/**
* @group legacy
* @expectedDeprecation Adding routes via the "Symfony\Bundle\FrameworkBundle\Tests\Kernel\MicroKernelWithConfigureRoutes:configureRoutes()" method is deprecated since Symfony 5.1 and will have no effect in 6.0; use "configureRouting()" instead.
* @expectedDeprecation Not overriding the "Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait::configureRouting()" method is deprecated since Symfony 5.1 and will trigger a fatal error in 6.0.
*/
public function testConfigureRoutingDeprecated()
{
$kernel = new MicroKernelWithConfigureRoutes('test', false);
$kernel->boot();
$kernel->handle(Request::create('/'));
}

public function test()
{
$kernel = new ConcreteMicroKernel('test', false);
Expand Down

This file was deleted.

2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Expand Up @@ -27,7 +27,7 @@
"symfony/polyfill-mbstring": "~1.0",
"symfony/filesystem": "^4.4|^5.0",
"symfony/finder": "^4.4|^5.0",
"symfony/routing": "^5.1"
"symfony/routing": "^5.0"
},
"require-dev": {
"doctrine/annotations": "~1.7",
Expand Down
1 change: 0 additions & 1 deletion src/Symfony/Component/Routing/CHANGELOG.md
Expand Up @@ -5,7 +5,6 @@ CHANGELOG
-----

* Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.
* Added support for a generic loader to `RoutingConfigurator`.

5.0.0
-----
Expand Down
Expand Up @@ -11,9 +11,7 @@

namespace Symfony\Component\Routing\Loader\Configurator;

use Symfony\Component\Config\Exception\LoaderLoadException;
use Symfony\Component\Config\Loader\FileLoader;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\Routing\Loader\PhpFileLoader;
use Symfony\Component\Routing\RouteCollection;

/**
Expand All @@ -27,7 +25,7 @@ class RoutingConfigurator
private $path;
private $file;

public function __construct(RouteCollection $collection, LoaderInterface $loader, ?string $path, string $file)
public function __construct(RouteCollection $collection, PhpFileLoader $loader, string $path, string $file)
{
$this->collection = $collection;
$this->loader = $loader;
Expand All @@ -40,7 +38,9 @@ public function __construct(RouteCollection $collection, LoaderInterface $loader
*/
final public function import($resource, string $type = null, bool $ignoreErrors = false, $exclude = null): ImportConfigurator
{
$imported = $this->load($resource, $type, $ignoreErrors, $exclude) ?: [];
$this->loader->setCurrentDir(\dirname($this->path));

$imported = $this->loader->import($resource, $type, $ignoreErrors, $this->file, $exclude) ?: [];
if (!\is_array($imported)) {
return new ImportConfigurator($this->collection, $imported);
}
Expand All @@ -57,34 +57,4 @@ final public function collection(string $name = ''): CollectionConfigurator
{
return new CollectionConfigurator($this->collection, $name);
}

/**
* @param string|string[]|null $exclude
*
* @return RouteCollection|RouteCollection[]|null
*/
private function load($resource, ?string $type, bool $ignoreErrors, $exclude)
{
$loader = $this->loader;

if (!$loader->supports($resource, $type)) {
if (null === $resolver = $loader->getResolver()) {
throw new LoaderLoadException($resource, $this->file, null, null, $type);
}

if (false === $loader = $resolver->resolve($resource, $type)) {
throw new LoaderLoadException($resource, $this->file, null, null, $type);
}
}

if (!$loader instanceof FileLoader) {
return $loader->load($resource, $type);
}

if (null !== $this->path) {
$this->loader->setCurrentDir(\dirname($this->path));
}

return $this->loader->import($resource, $type, $ignoreErrors, $this->file, $exclude);
}
}
3 changes: 3 additions & 0 deletions src/Symfony/Component/Routing/RouteCollectionBuilder.php
Expand Up @@ -14,6 +14,9 @@
use Symfony\Component\Config\Exception\LoaderLoadException;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\Config\Resource\ResourceInterface;
use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;

@trigger_error(sprintf('The "%s" class is deprecated since Symfony 5.1, use "%s" instead.', RouteCollectionBuilder::class, RoutingConfigurator::class), E_USER_DEPRECATED);

/**
* Helps add and import routes into a RouteCollection.
Expand Down

0 comments on commit 8c80c5b

Please sign in to comment.