Skip to content

Commit

Permalink
feature #32937 [Routing] Deprecate RouteCollectionBuilder (vudaltsov)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 5.1-dev branch (closes #32937).

Discussion
----------

[Routing] Deprecate RouteCollectionBuilder

| Q             | A
| ------------- | ---
| Branch?       | 5.1
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | #32240
| License       | MIT
| Doc PR        | symfony/symfony-docs#12688
| Recipe PR | symfony/recipes#690

A lot to be done here after the implementation is accepted:
- [x] finish deprecations in the MicroKernelTrait
- [x] deprecate the class
- [x] mention in the CHANGELOG file
- [x] mention in the UPGRADE file
- [x] mark tests as legacy
- [x] add a doc PR
- [x] update the recipe

Ping @Tobion , @nicolas-grekas .

Commits
-------

e641cbd [Routing] Deprecate RouteCollectionBuilder
  • Loading branch information
fabpot committed Nov 25, 2019
2 parents 3888312 + e641cbd commit 7719fc7
Show file tree
Hide file tree
Showing 12 changed files with 200 additions and 12 deletions.
13 changes: 13 additions & 0 deletions UPGRADE-5.1.md
@@ -0,0 +1,13 @@
UPGRADE FROM 5.0 to 5.1
=======================

FrameworkBundle
---------------

* Marked `MicroKernelTrait::configureRoutes()` as `@internal` and `@final`.
* Deprecated not overriding `MicroKernelTrait::configureRouting()`.

Routing
-------

* Deprecated `RouteCollectionBuilder` in favor of `RoutingConfigurator`.
13 changes: 13 additions & 0 deletions UPGRADE-6.0.md
@@ -0,0 +1,13 @@
UPGRADE FROM 5.x to 6.0
=======================

FrameworkBundle
---------------

* Removed `MicroKernelTrait::configureRoutes()`.
* Made `MicroKernelTrait::configureRouting()` abstract.

Routing
-------

* Removed `RouteCollectionBuilder`.
6 changes: 6 additions & 0 deletions src/Symfony/Bundle/FrameworkBundle/CHANGELOG.md
@@ -1,6 +1,12 @@
CHANGELOG
=========

5.1.0
-----

* Marked `MicroKernelTrait::configureRoutes()` as `@internal` and `@final`.
* Deprecated not overriding `MicroKernelTrait::configureRouting()`.

5.0.0
-----

Expand Down
33 changes: 31 additions & 2 deletions src/Symfony/Bundle/FrameworkBundle/Kernel/MicroKernelTrait.php
Expand Up @@ -14,6 +14,7 @@
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;
use Symfony\Component\Routing\RouteCollectionBuilder;

/**
Expand All @@ -29,8 +30,28 @@ trait MicroKernelTrait
*
* $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.
*
* $routes->import($this->getProjectDir().'/config/*.{yaml,php}');
* $routes
* ->add('admin_dashboard', '/admin')
* ->controller('App\Controller\AdminController::dashboard')
* ;
*/
abstract protected function configureRoutes(RouteCollectionBuilder $routes);
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);
}

/**
* Configures the container.
Expand Down Expand Up @@ -91,7 +112,15 @@ public function loadRoutes(LoaderInterface $loader)
{
$routes = new RouteCollectionBuilder($loader);
$this->configureRoutes($routes);
$collection = $routes->build();

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);
}

$file = (new \ReflectionObject($this))->getFileName();
$this->configureRouting(new RoutingConfigurator($collection, $loader, null, $file));

return $routes->build();
return $collection;
}
}
Expand Up @@ -22,7 +22,7 @@
use Symfony\Component\HttpKernel\Event\ExceptionEvent;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\HttpKernel\KernelEvents;
use Symfony\Component\Routing\RouteCollectionBuilder;
use Symfony\Component\Routing\Loader\Configurator\RoutingConfigurator;

class ConcreteMicroKernel extends Kernel implements EventSubscriberInterface
{
Expand Down Expand Up @@ -80,10 +80,10 @@ public function __destruct()
$fs->remove($this->cacheDir);
}

protected function configureRoutes(RouteCollectionBuilder $routes)
protected function configureRouting(RoutingConfigurator $routes): void
{
$routes->add('/', 'kernel::halloweenAction');
$routes->add('/danger', 'kernel::dangerousAction');
$routes->add('halloween', '/')->controller('kernel::halloweenAction');
$routes->add('danger', '/danger')->controller('kernel::dangerousAction');
}

protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader)
Expand Down
Expand Up @@ -19,6 +19,18 @@

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
@@ -0,0 +1,74 @@
<?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\Bundle\FrameworkBundle\Tests\Kernel;

use Psr\Log\NullLogger;
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait;
use Symfony\Component\Config\Loader\LoaderInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\Filesystem\Filesystem;
use Symfony\Component\HttpKernel\Kernel;
use Symfony\Component\Routing\RouteCollectionBuilder;

class MicroKernelWithConfigureRoutes extends Kernel
{
use MicroKernelTrait;

private $cacheDir;

public function registerBundles(): iterable
{
return [
new FrameworkBundle(),
];
}

public function getCacheDir(): string
{
return $this->cacheDir = sys_get_temp_dir().'/sf_micro_kernel_with_configured_routes';
}

public function getLogDir(): string
{
return $this->cacheDir;
}

public function __sleep(): array
{
throw new \BadMethodCallException('Cannot serialize '.__CLASS__);
}

public function __wakeup()
{
throw new \BadMethodCallException('Cannot unserialize '.__CLASS__);
}

public function __destruct()
{
$fs = new Filesystem();
$fs->remove($this->cacheDir);
}

protected function configureRoutes(RouteCollectionBuilder $routes)
{
$routes->add('/', 'kernel::halloweenAction');
}

protected function configureContainer(ContainerBuilder $c, LoaderInterface $loader)
{
$c->register('logger', NullLogger::class);
$c->loadFromExtension('framework', [
'secret' => '$ecret',
]);
}
}
2 changes: 1 addition & 1 deletion src/Symfony/Bundle/FrameworkBundle/composer.json
Expand Up @@ -26,7 +26,7 @@
"symfony/polyfill-mbstring": "~1.0",
"symfony/filesystem": "^4.4|^5.0",
"symfony/finder": "^4.4|^5.0",
"symfony/routing": "^5.0"
"symfony/routing": "^5.1"
},
"require-dev": {
"doctrine/annotations": "~1.7",
Expand Down
6 changes: 6 additions & 0 deletions src/Symfony/Component/Routing/CHANGELOG.md
@@ -1,6 +1,12 @@
CHANGELOG
=========

5.1.0
-----

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

5.0.0
-----

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

namespace Symfony\Component\Routing\Loader\Configurator;

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

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

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

$imported = $this->loader->import($resource, $type, $ignoreErrors, $this->file, $exclude) ?: [];
$imported = $this->load($resource, $type, $ignoreErrors, $exclude) ?: [];
if (!\is_array($imported)) {
return new ImportConfigurator($this->collection, $imported);
}
Expand All @@ -57,4 +57,34 @@ 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);
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Component/Routing/RouteCollectionBuilder.php
Expand Up @@ -19,6 +19,8 @@
* Helps add and import routes into a RouteCollection.
*
* @author Ryan Weaver <ryan@knpuniversity.com>
*
* @deprecated since Symfony 5.1, use RoutingConfigurator instead
*/
class RouteCollectionBuilder
{
Expand Down
Expand Up @@ -19,6 +19,9 @@
use Symfony\Component\Routing\RouteCollection;
use Symfony\Component\Routing\RouteCollectionBuilder;

/**
* @group legacy
*/
class RouteCollectionBuilderTest extends TestCase
{
public function testImport()
Expand Down

0 comments on commit 7719fc7

Please sign in to comment.