From c96914055aa2e40a9a7cf9006154167fd3516f30 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Sun, 19 Apr 2020 13:58:08 +0200 Subject: [PATCH] more places to support the route object parameter --- CHANGELOG.md | 4 +- src/ChainRouter.php | 2 +- src/ContentAwareGenerator.php | 29 +++++- src/DynamicRouter.php | 2 +- src/ProviderBasedGenerator.php | 20 ++++ src/VersatileGeneratorInterface.php | 17 ++-- tests/Unit/Routing/ChainRouterTest.php | 22 +++-- .../Routing/ContentAwareGeneratorTest.php | 92 +++++++++++++++++-- tests/Unit/Routing/DynamicRouterTest.php | 3 + 9 files changed, 158 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54b8b4d3..4aac044c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,8 @@ Changelog name `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` (`cmf_routing_object`) and pass the route object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT` (`_route_object`). -* The VersatileGeneratorInterface is deprecated as it was used to avoid errors - with routers not supporting objects in `$name`. +* The VersatileGeneratorInterface::supports method is deprecated as it was used + to avoid errors with routers not supporting objects in `$name`. 2.2.0 ----- diff --git a/src/ChainRouter.php b/src/ChainRouter.php index 9be87d18..87b892d7 100644 --- a/src/ChainRouter.php +++ b/src/ChainRouter.php @@ -219,7 +219,7 @@ private function doMatch($pathinfo, Request $request = null) public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH) { if (is_object($name)) { - @trigger_error(sprintf('Passing an object as the route name is deprecated in symfony-cmf/Routing v2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` constant as the route name and the object as "%s" parameter in the parameters array.', RouteObjectInterface::ROUTE_OBJECT), E_USER_DEPRECATED); + @trigger_error('Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`.', E_USER_DEPRECATED); } $debug = []; diff --git a/src/ContentAwareGenerator.php b/src/ContentAwareGenerator.php index 7d213cbc..d207c356 100644 --- a/src/ContentAwareGenerator.php +++ b/src/ContentAwareGenerator.php @@ -69,9 +69,15 @@ public function setContentRepository(ContentRepositoryInterface $contentReposito public function generate($name, $parameters = [], $absolute = UrlGeneratorInterface::ABSOLUTE_PATH) { if ($name instanceof SymfonyRoute) { + @trigger_error('Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`.', E_USER_DEPRECATED); + $route = $this->getBestLocaleRoute($name, $parameters); - } elseif (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof SymfonyRoute) { - $route = $this->getBestLocaleRoute($parameters[RouteObjectInterface::ROUTE_OBJECT], $parameters); + } elseif (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name) { + if (array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof SymfonyRoute) { + $route = $this->getBestLocaleRoute($parameters[RouteObjectInterface::ROUTE_OBJECT], $parameters); + } else { + $route = $this->getRouteByContent($name, $parameters); + } } elseif (is_string($name) && $name) { $route = $this->getRouteByName($name, $parameters); } else { @@ -166,8 +172,13 @@ protected function getBestLocaleRoute(SymfonyRoute $route, $parameters) protected function getRouteByContent($name, &$parameters) { if ($name instanceof RouteReferrersReadInterface) { + @trigger_error('Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`.', E_USER_DEPRECATED); + $content = $name; - } elseif (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof RouteReferrersReadInterface) { + } elseif (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name + && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) + && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof RouteReferrersReadInterface + ) { $content = $parameters[RouteObjectInterface::ROUTE_OBJECT]; } elseif (array_key_exists('content_id', $parameters) && null !== $this->contentRepository @@ -294,10 +305,20 @@ public function supports($name) */ public function getRouteDebugMessage($name, array $parameters = []) { - if (!$name && array_key_exists('content_id', $parameters)) { + if ((!$name || RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name) + && array_key_exists('content_id', $parameters) + ) { return 'Content id '.$parameters['content_id']; } + if (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name + && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) + && $parameters[RouteObjectInterface::ROUTE_OBJECT] instanceof RouteReferrersReadInterface + ) { + return 'Route aware content '.parent::getRouteDebugMessage($name, $parameters); + } + + // legacy if ($name instanceof RouteReferrersReadInterface) { return 'Route aware content '.parent::getRouteDebugMessage($name, $parameters); } diff --git a/src/DynamicRouter.php b/src/DynamicRouter.php index 202703f7..b96d5dbb 100644 --- a/src/DynamicRouter.php +++ b/src/DynamicRouter.php @@ -175,7 +175,7 @@ public function getGenerator() public function generate($name, $parameters = [], $referenceType = UrlGeneratorInterface::ABSOLUTE_PATH) { if (is_object($name)) { - @trigger_error(sprintf('Passing an object as the route name is deprecated in symfony-cmf/Routing v2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` constant as the route name and the object as "%s" parameter in the parameters array.', RouteObjectInterface::ROUTE_OBJECT), E_USER_DEPRECATED); + @trigger_error('Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT', E_USER_DEPRECATED); } if ($this->eventDispatcher) { diff --git a/src/ProviderBasedGenerator.php b/src/ProviderBasedGenerator.php index d4028c23..b850c7d2 100644 --- a/src/ProviderBasedGenerator.php +++ b/src/ProviderBasedGenerator.php @@ -77,10 +77,30 @@ public function supports($name) */ public function getRouteDebugMessage($name, array $parameters = []) { + if (RouteObjectInterface::OBJECT_BASED_ROUTE_NAME === $name + && array_key_exists(RouteObjectInterface::ROUTE_OBJECT, $parameters) + ) { + $routeObject = $parameters[RouteObjectInterface::ROUTE_OBJECT]; + if ($routeObject instanceof RouteObjectInterface) { + return 'Route with key '.$routeObject->getRouteKey(); + } + + if ($routeObject instanceof SymfonyRoute) { + return 'Route with path '.$routeObject->getPath(); + } + + if (is_object($routeObject)) { + return get_class($routeObject); + } + + return 'Null route'; + } + if (is_scalar($name)) { return $name; } + // legacy if (is_array($name)) { return serialize($name); } diff --git a/src/VersatileGeneratorInterface.php b/src/VersatileGeneratorInterface.php index d20f7836..66fa368f 100644 --- a/src/VersatileGeneratorInterface.php +++ b/src/VersatileGeneratorInterface.php @@ -14,10 +14,7 @@ use Symfony\Component\Routing\Generator\UrlGeneratorInterface; /** - * This generator is able to handle more than string route names as symfony - * core supports them. - * - * @deprecated The "Symfony\Cmf\Component\Routing\VersatileGeneratorInterface" is deprecated in symfony-cmf/Routing v2.3 and will be removed in symfony-cmf/Routing v3.O. Use the "_route_object" parameter instead to handle route objects + * This generator can provide additional information about the route that we wanted to generate. */ interface VersatileGeneratorInterface extends UrlGeneratorInterface { @@ -28,6 +25,13 @@ interface VersatileGeneratorInterface extends UrlGeneratorInterface * resolved to a route, only whether the router can generate routes from * objects of this class. * + * @deprecated This method is deprecated since version 2.3 and will be + * removed in version 3.O. + * + * This method was used to not call generators that can not handle objects + * in $name. With Symfony 5, this becomes obsolete as the strict type + * declaration prevents passing anything else than a string as $name. + * * @param mixed $name The route "name" which may also be an object or anything * * @return bool @@ -38,9 +42,8 @@ public function supports($name); * Convert a route identifier (name, content object etc) into a string * usable for logging and other debug/error messages. * - * @param mixed $name - * @param array $parameters which should contain a content field containing - * a RouteReferrersReadInterface object + * @param mixed $name In Symfony 5, the name can only be a string + * @param array $parameters Which might hold a route object or content id or similar to include in the debug message * * @return string */ diff --git a/tests/Unit/Routing/ChainRouterTest.php b/tests/Unit/Routing/ChainRouterTest.php index bf944481..cf60fd97 100644 --- a/tests/Unit/Routing/ChainRouterTest.php +++ b/tests/Unit/Routing/ChainRouterTest.php @@ -617,12 +617,12 @@ public function testGenerateNotFound() * Route is an object but no versatile generator around to do the debug message. * * @group legacy - * @expectedDeprecation Passing an object as the route name is deprecated in symfony-cmf/Routing v2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` constant as the route name and the object as "_route_object" parameter in the parameters array. + * @expectedDeprecation Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. */ public function testGenerateObjectNotFound() { if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Skip this test on >= sf5. This will throw a \TypeError.'); + $this->markTestSkipped('Symfony 5 would throw a TypeError.'); } $name = new \stdClass(); @@ -645,12 +645,12 @@ public function testGenerateObjectNotFound() * A versatile router will generate the debug message. * * @group legacy - * @expectedDeprecation Passing an object as the route name is deprecated in symfony-cmf/Routing v2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` constant as the route name and the object as "_route_object" parameter in the parameters array. + * @expectedDeprecation Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. */ public function testGenerateObjectNotFoundVersatile() { if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Skip this test on >= sf5. This will throw a \TypeError.'); + $this->markTestSkipped('Symfony 5 would throw a TypeError.'); } $name = new \stdClass(); @@ -681,12 +681,12 @@ public function testGenerateObjectNotFoundVersatile() /** * @group legacy - * @expectedDeprecation Passing an object as the route name is deprecated in symfony-cmf/Routing v2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` constant as the route name and the object as "_route_object" parameter in the parameters array. + * @expectedDeprecation Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. */ public function testGenerateObjectName() { if (!class_exists(ObjectRouteLoader::class)) { - $this->markTestSkipped('Skip this test on >= sf5. This will throw a \TypeError.'); + $this->markTestSkipped('Symfony 5 would throw a TypeError.'); } $name = new \stdClass(); @@ -718,6 +718,9 @@ public function testGenerateObjectName() $this->assertEquals($name, $result); } + /** + * This test currently triggers a deprecation notice because of ChainRouter BC. + */ public function testGenerateWithObjectNameInParametersNotFoundVersatile() { $name = RouteObjectInterface::OBJECT_BASED_ROUTE_NAME; @@ -757,13 +760,13 @@ public function testGenerateWithObjectNameInParameters() ->expects($this->once()) ->method('generate') ->with($name, $parameters, UrlGeneratorInterface::ABSOLUTE_PATH) - ->willReturn($name) + ->willReturn('/foo/bar') ; $this->router->add($defaultRouter, 200); $result = $this->router->generate($name, $parameters); - $this->assertEquals($name, $result); + $this->assertEquals('/foo/bar', $result); } public function testWarmup() @@ -817,6 +820,9 @@ public function testRouteCollection() $this->assertEquals(['high', 'low'], $names); } + /** + * @group legacy + */ public function testSupport() { $router = $this->createMock(VersatileRouter::class); diff --git a/tests/Unit/Routing/ContentAwareGeneratorTest.php b/tests/Unit/Routing/ContentAwareGeneratorTest.php index 889e8df7..447a7a97 100644 --- a/tests/Unit/Routing/ContentAwareGeneratorTest.php +++ b/tests/Unit/Routing/ContentAwareGeneratorTest.php @@ -21,6 +21,7 @@ use Symfony\Cmf\Component\Routing\Tests\Unit\Routing\RouteMock; use Symfony\Component\Routing\CompiledRoute; use Symfony\Component\Routing\Exception\RouteNotFoundException; +use Symfony\Component\Routing\Loader\ObjectRouteLoader; use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\Route; @@ -71,8 +72,16 @@ public function setUp() $this->generator = new TestableContentAwareGenerator($this->provider); } + /** + * @group legacy + * @expectedDeprecation Passing an object as route name is deprecated since version 2.3 and will not work in Symfony 5.0. Pass the `RouteObjectInterface::OBJECT_BASED_ROUTE_NAME` as route name and the object in the parameters with key `RouteObjectInterface::ROUTE_OBJECT`. + */ public function testGenerateFromContent() { + if (!class_exists(ObjectRouteLoader::class)) { + $this->markTestSkipped('Symfony 5 would throw a TypeError.'); + } + $this->provider->expects($this->never()) ->method('getRouteByName') ; @@ -101,7 +110,7 @@ public function testGenerateFromContentInParameters() $this->assertEquals('result_url', $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, [RouteObjectInterface::ROUTE_OBJECT => $this->routeDocument])); } - public function testGenerateFromContentId() + public function testGenerateFromContentIdEmptyRouteName() { $this->provider->expects($this->never()) ->method('getRouteByName') @@ -128,6 +137,34 @@ public function testGenerateFromContentId() $this->assertEquals('result_url', $this->generator->generate('', ['content_id' => '/content/id'])); } + public function testGenerateFromContentId() + { + $this->provider->expects($this->never()) + ->method('getRouteByName') + ; + + $contentRepository = $this->createMock(ContentRepositoryInterface::class); + $contentRepository->expects($this->once()) + ->method('findById') + ->with('/content/id') + ->will($this->returnValue($this->contentDocument)) + ; + $this->generator->setContentRepository($contentRepository); + + $this->contentDocument->expects($this->once()) + ->method('getRoutes') + ->will($this->returnValue([$this->routeDocument])) + ; + + $this->routeDocument->expects($this->once()) + ->method('compile') + ->will($this->returnValue($this->routeCompiled)) + ; + + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['content_id' => '/content/id']); + $this->assertEquals('result_url', $generated); + } + public function testGenerateEmptyRouteString() { $this->provider->expects($this->never()) @@ -144,7 +181,8 @@ public function testGenerateEmptyRouteString() ->will($this->returnValue($this->routeCompiled)) ; - $this->assertEquals('result_url', $this->generator->generate($this->contentDocument)); + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, [RouteObjectInterface::ROUTE_OBJECT => $this->contentDocument]); + $this->assertEquals('result_url', $generated); } public function testGenerateRouteMultilang() @@ -174,7 +212,8 @@ public function testGenerateRouteMultilang() ->will($this->returnValue($this->routeCompiled)) ; - $this->assertEquals('result_url', $this->generator->generate($route_en, ['_locale' => 'de'])); + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['_locale' => 'de', RouteObjectInterface::ROUTE_OBJECT => $route_en]); + $this->assertEquals('result_url', $generated); } public function testGenerateRouteMultilangDefaultLocale() @@ -199,7 +238,8 @@ public function testGenerateRouteMultilangDefaultLocale() ->will($this->returnValue([])) ; - $this->assertEquals('result_url', $this->generator->generate($route, ['_locale' => 'en'])); + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['_locale' => 'en', RouteObjectInterface::ROUTE_OBJECT => $route]); + $this->assertEquals('result_url', $generated); } public function testGenerateRouteMultilangLocaleNomatch() @@ -228,7 +268,8 @@ public function testGenerateRouteMultilangLocaleNomatch() ->method('compile') ; - $this->assertEquals('result_url', $this->generator->generate($route_en, ['_locale' => 'fr'])); + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['_locale' => 'fr', RouteObjectInterface::ROUTE_OBJECT => $route_en]); + $this->assertEquals('result_url', $generated); } public function testGenerateNoncmfRouteMultilang() @@ -240,7 +281,8 @@ public function testGenerateNoncmfRouteMultilang() ->will($this->returnValue($this->routeCompiled)) ; - $this->assertEquals('result_url', $this->generator->generate($route_en, ['_locale' => 'de'])); + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['_locale' => 'de', RouteObjectInterface::ROUTE_OBJECT => $route_en]); + $this->assertEquals('result_url', $generated); } public function testGenerateRoutenameMultilang() @@ -314,7 +356,8 @@ public function testGenerateDocumentMultilang() ->will($this->returnValue($this->routeCompiled)) ; - $this->assertEquals('result_url', $this->generator->generate($this->contentDocument, ['_locale' => 'de'])); + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['_locale' => 'de', RouteObjectInterface::ROUTE_OBJECT => $this->contentDocument]); + $this->assertEquals('result_url', $generated); } public function testGenerateDocumentMultilangLocaleNomatch() @@ -336,7 +379,8 @@ public function testGenerateDocumentMultilangLocaleNomatch() ->method('compile') ; - $this->assertEquals('result_url', $this->generator->generate($this->contentDocument, ['_locale' => 'fr'])); + $generated = $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['_locale' => 'fr', RouteObjectInterface::ROUTE_OBJECT => $this->contentDocument]); + $this->assertEquals('result_url', $generated); } /** @@ -353,6 +397,21 @@ public function testGenerateNoContent() */ public function testGenerateInvalidContent() { + $this->expectException(RouteNotFoundException::class); + $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, [RouteObjectInterface::ROUTE_OBJECT => $this]); + } + + /** + * Generate with an object that is neither a route nor route aware. + * + * @group legacy + */ + public function testGenerateInvalidContentLegacy() + { + if (!class_exists(ObjectRouteLoader::class)) { + $this->markTestSkipped('Symfony 5 would throw a TypeError.'); + } + $this->expectException(RouteNotFoundException::class); $this->generator->generate($this); } @@ -422,7 +481,7 @@ public function testGenerateNoRoutes() ->will($this->returnValue([])); $this->expectException(RouteNotFoundException::class); - $this->generator->generate($this->contentDocument); + $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, [RouteObjectInterface::ROUTE_OBJECT => $this->contentDocument]); } /** @@ -435,7 +494,7 @@ public function testGenerateInvalidRoute() ->will($this->returnValue([$this])); $this->expectException(RouteNotFoundException::class); - $this->generator->generate($this->contentDocument); + $this->generator->generate(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, [RouteObjectInterface::ROUTE_OBJECT => $this->contentDocument]); } public function testGetLocaleAttribute() @@ -464,6 +523,9 @@ public function testGetLocaleContext() $this->assertEquals('de', $this->generator->getLocale($attributes)); } + /** + * @group legacy + */ public function testSupports() { $this->assertTrue($this->generator->supports('')); @@ -473,6 +535,16 @@ public function testSupports() } public function testGetRouteDebugMessage() + { + $this->assertContains('/some/content', $this->generator->getRouteDebugMessage(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, ['content_id' => '/some/content'])); + $this->assertContains('Route aware content Symfony\Cmf\Component\Routing\Tests\Routing\RouteAware', $this->generator->getRouteDebugMessage(RouteObjectInterface::OBJECT_BASED_ROUTE_NAME, [RouteObjectInterface::ROUTE_OBJECT => new RouteAware()])); + $this->assertContains('/some/content', $this->generator->getRouteDebugMessage('/some/content')); + } + + /** + * @legacy + */ + public function testGetRouteDebugMessageLegacy() { $this->assertContains('/some/content', $this->generator->getRouteDebugMessage(null, ['content_id' => '/some/content'])); $this->assertContains('Route aware content Symfony\Cmf\Component\Routing\Tests\Routing\RouteAware', $this->generator->getRouteDebugMessage(new RouteAware())); diff --git a/tests/Unit/Routing/DynamicRouterTest.php b/tests/Unit/Routing/DynamicRouterTest.php index 6824da9c..02aa03a2 100644 --- a/tests/Unit/Routing/DynamicRouterTest.php +++ b/tests/Unit/Routing/DynamicRouterTest.php @@ -138,6 +138,9 @@ public function testGenerate() $this->assertEquals('http://test', $url); } + /** + * @group legacy + */ public function testSupports() { $name = 'foo/bar';