-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feature #32582 [Routing] Deprecate ServiceRouterLoader and ObjectRout…
…eLoader in favor of ContainerLoader and ObjectLoader (fancyweb) This PR was merged into the 4.4 branch. Discussion ---------- [Routing] Deprecate ServiceRouterLoader and ObjectRouteLoader in favor of ContainerLoader and ObjectLoader | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | yes | Tests pass? | yes | Fixed tickets | #30926 (comment) | License | MIT | Doc PR | - This PR aims at deprecating some things to have a more consistent code. ### ServiceRouterLoader 1. This class actually fetches an object from a container. In #30926 (comment), it was suggested that it should be renamed to `ContainerRouteLoader`. Actually I think it's better to rename it to `ContainerLoader` since all others route loaders does not have "Route" in their name. Let's be consistent! 2. This class is in a `DependencyInjection` sub directory for historical reasons. Let's remove that! It accepts any PSR-11 container. ### ObjectRouteLoader 1. This class has "Route" in its name too. Let's rename it! 2. This class is supposed to be an abstract implementation for "object" loaders to reuse, but in its code it has a lot of references to "services". Let's remove those references! That means renaming some methods, altering messages, etc.. That also means removing the `supports` method from it to let extending classes implement it. 3. IMHO, this abstract implementation is useless. We sould just deprecate the whole class and move the implemention in the `ContainerLoader` class. Commits ------- 1548101 [Routing] Deprecate ServiceRouterLoader and ObjectRouteLoader in favor of ContainerLoader and ObjectLoader
- Loading branch information
Showing
14 changed files
with
398 additions
and
57 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
<?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; | ||
|
||
use Psr\Container\ContainerInterface; | ||
|
||
/** | ||
* A route loader that executes a service from a PSR-11 container to load the routes. | ||
* | ||
* @author Ryan Weaver <ryan@knpuniversity.com> | ||
*/ | ||
class ContainerLoader extends ObjectLoader | ||
{ | ||
private $container; | ||
|
||
public function __construct(ContainerInterface $container) | ||
{ | ||
$this->container = $container; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
public function supports($resource, $type = null) | ||
{ | ||
return 'service' === $type; | ||
} | ||
|
||
/** | ||
* {@inheritdoc} | ||
*/ | ||
protected function getObject(string $id) | ||
{ | ||
return $this->container->get($id); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
<?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; | ||
|
||
use Symfony\Component\Config\Loader\Loader; | ||
use Symfony\Component\Config\Resource\FileResource; | ||
use Symfony\Component\Routing\RouteCollection; | ||
|
||
/** | ||
* A route loader that calls a method on an object to load the routes. | ||
* | ||
* @author Ryan Weaver <ryan@knpuniversity.com> | ||
*/ | ||
abstract class ObjectLoader extends Loader | ||
{ | ||
/** | ||
* Returns the object that the method will be called on to load routes. | ||
* | ||
* For example, if your application uses a service container, | ||
* the $id may be a service id. | ||
* | ||
* @return object | ||
*/ | ||
abstract protected function getObject(string $id); | ||
|
||
/** | ||
* Calls the object method that will load the routes. | ||
* | ||
* @param string $resource object_id::method | ||
* @param string|null $type The resource type | ||
* | ||
* @return RouteCollection | ||
*/ | ||
public function load($resource, $type = null) | ||
{ | ||
if (!preg_match('/^[^\:]+(?:::(?:[^\:]+))?$/', $resource)) { | ||
throw new \InvalidArgumentException(sprintf('Invalid resource "%s" passed to the %s route loader: use the format "object_id::method" or "object_id" if your object class has an "__invoke" method.', $resource, \is_string($type) ? '"'.$type.'"' : 'object')); | ||
} | ||
|
||
$parts = explode('::', $resource); | ||
$method = $parts[1] ?? '__invoke'; | ||
|
||
$loaderObject = $this->getObject($parts[0]); | ||
|
||
if (!\is_object($loaderObject)) { | ||
throw new \LogicException(sprintf('%s:getObject() must return an object: %s returned', \get_class($this), \gettype($loaderObject))); | ||
} | ||
|
||
if (!\is_callable([$loaderObject, $method])) { | ||
throw new \BadMethodCallException(sprintf('Method "%s" not found on "%s" when importing routing resource "%s"', $method, \get_class($loaderObject), $resource)); | ||
} | ||
|
||
$routeCollection = $loaderObject->$method($this); | ||
|
||
if (!$routeCollection instanceof RouteCollection) { | ||
$type = \is_object($routeCollection) ? \get_class($routeCollection) : \gettype($routeCollection); | ||
|
||
throw new \LogicException(sprintf('The %s::%s method must return a RouteCollection: %s returned', \get_class($loaderObject), $method, $type)); | ||
} | ||
|
||
// make the object file tracked so that if it changes, the cache rebuilds | ||
$this->addClassResource(new \ReflectionClass($loaderObject), $routeCollection); | ||
|
||
return $routeCollection; | ||
} | ||
|
||
private function addClassResource(\ReflectionClass $class, RouteCollection $collection) | ||
{ | ||
do { | ||
if (is_file($class->getFileName())) { | ||
$collection->addResource(new FileResource($class->getFileName())); | ||
} | ||
} while ($class = $class->getParentClass()); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
src/Symfony/Component/Routing/Tests/Fixtures/TestObjectRouteLoader.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<?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\Fixtures; | ||
|
||
use Symfony\Component\Routing\Loader\ObjectRouteLoader; | ||
|
||
class TestObjectRouteLoader extends ObjectRouteLoader | ||
{ | ||
public $loaderMap = []; | ||
|
||
protected function getServiceObject($id) | ||
{ | ||
return $this->loaderMap[$id] ?? null; | ||
} | ||
} |
36 changes: 36 additions & 0 deletions
36
src/Symfony/Component/Routing/Tests/Loader/ContainerLoaderTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
<?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 Symfony\Component\DependencyInjection\Container; | ||
use Symfony\Component\Routing\Loader\ContainerLoader; | ||
|
||
class ContainerLoaderTest extends TestCase | ||
{ | ||
/** | ||
* @dataProvider supportsProvider | ||
*/ | ||
public function testSupports(bool $expected, string $type = null) | ||
{ | ||
$this->assertSame($expected, (new ContainerLoader(new Container()))->supports('foo', $type)); | ||
} | ||
|
||
public function supportsProvider() | ||
{ | ||
return [ | ||
[true, 'service'], | ||
[false, 'bar'], | ||
[false, null], | ||
]; | ||
} | ||
} |
29 changes: 29 additions & 0 deletions
29
src/Symfony/Component/Routing/Tests/Loader/DependencyInjection/ServiceRouterLoaderTest.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?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 Symfony\Component\DependencyInjection\Container; | ||
use Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoader; | ||
|
||
class ServiceRouterLoaderTest extends TestCase | ||
{ | ||
/** | ||
* @group legacy | ||
* @expectedDeprecation The "Symfony\Component\Routing\Loader\DependencyInjection\ServiceRouterLoader" class is deprecated since Symfony 4.4, use "Symfony\Component\Routing\Loader\ContainerLoader" instead. | ||
* @expectedDeprecation The "Symfony\Component\Routing\Loader\ObjectRouteLoader" class is deprecated since Symfony 4.4, use "Symfony\Component\Routing\Loader\ObjectLoader" instead. | ||
*/ | ||
public function testDeprecationWarning() | ||
{ | ||
new ServiceRouterLoader(new Container()); | ||
} | ||
} |
Oops, something went wrong.