Skip to content

[Routing] Fix localized paths #40767

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

Merged
merged 1 commit into from
May 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/Symfony/Component/Routing/Annotation/Route.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,19 @@ public function __construct(
} elseif (!\is_array($data)) {
throw new \TypeError(sprintf('"%s": Argument $data is expected to be a string or array, got "%s".', __METHOD__, get_debug_type($data)));
} elseif ([] !== $data) {
trigger_deprecation('symfony/routing', '5.3', 'Passing an array as first argument to "%s" is deprecated. Use named arguments instead.', __METHOD__);
$deprecation = false;
foreach ($data as $key => $val) {
if (\in_array($key, ['path', 'name', 'requirements', 'options', 'defaults', 'host', 'methods', 'schemes', 'condition', 'priority', 'locale', 'format', 'utf8', 'stateless', 'env', 'value'])) {
$deprecation = true;
}
}

if ($deprecation) {
trigger_deprecation('symfony/routing', '5.3', 'Passing an array as first argument to "%s" is deprecated. Use named arguments instead.', __METHOD__);
} else {
$localizedPaths = $data;
$data = ['path' => $localizedPaths];
}
}
if (null !== $path && !\is_string($path) && !\is_array($path)) {
throw new \TypeError(sprintf('"%s": Argument $path is expected to be a string, array or null, got "%s".', __METHOD__, get_debug_type($path)));
Expand Down
93 changes: 59 additions & 34 deletions src/Symfony/Component/Routing/Tests/Annotation/RouteTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,73 +11,98 @@

namespace Symfony\Component\Routing\Tests\Annotation;

use Doctrine\Common\Annotations\AnnotationReader;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures\FooController;
use Symfony\Component\Routing\Tests\Fixtures\AttributeFixtures\FooController as FooAttributesController;

class RouteTest extends TestCase
{
use ExpectDeprecationTrait;

/**
* @group legacy
*/
public function testInvalidRouteParameter()
private function getMethodAnnotation(string $method, bool $attributes): Route
{
$this->expectException(\BadMethodCallException::class);
new Route(['foo' => 'bar']);
$class = $attributes ? FooAttributesController::class : FooController::class;
$reflection = new \ReflectionMethod($class, $method);

if ($attributes) {
$attributes = $reflection->getAttributes(Route::class);
$route = $attributes[0]->newInstance();
} else {
$reader = new AnnotationReader();
$route = $reader->getMethodAnnotation($reflection, Route::class);
}

if (!$route instanceof Route) {
throw new \Exception('Can\'t parse annotation');
}

return $route;
}

public function provideDeprecationArrayAsFirstArgument()
{
return [
['requirements', ['locale' => 'en'], 'getRequirements'],
['options', ['compiler_class' => 'RouteCompiler'], 'getOptions'],
['name', 'blog_index', 'getName'],
['defaults', ['_controller' => 'MyBlogBundle:Blog:index'], 'getDefaults'],
['schemes', ['https'], 'getSchemes'],
['methods', ['GET', 'POST'], 'getMethods'],
['host', '{locale}.example.com', 'getHost'],
['condition', 'context.getMethod() == "GET"', 'getCondition'],
['value', '/Blog', 'getPath'],
['value', ['nl' => '/hier', 'en' => '/here'], 'getLocalizedPaths'],
];
}

/**
* @group legacy
* @dataProvider provideDeprecationArrayAsFirstArgument
*/
public function testTryingToSetLocalesDirectly()
public function testDeprecationArrayAsFirstArgument(string $parameter, $value, string $getter)
{
$this->expectException(\BadMethodCallException::class);
new Route(['locales' => ['nl' => 'bar']]);
$this->expectDeprecation('Since symfony/routing 5.3: Passing an array as first argument to "Symfony\Component\Routing\Annotation\Route::__construct" is deprecated. Use named arguments instead.');

$route = new Route([$parameter => $value]);
$this->assertEquals($route->$getter(), $value);
}

/**
* @requires PHP 8
* @dataProvider getValidParameters
*/
public function testRouteParameters(string $parameter, $value, string $getter)
public function testRouteParameters(string $methodName, string $getter, $expectedReturn)
{
$route = new Route(...[$parameter => $value]);
$this->assertEquals($route->$getter(), $value);
$route = $this->getMethodAnnotation($methodName, true);
$this->assertEquals($route->$getter(), $expectedReturn);
}

/**
* @group legacy
* @dataProvider getLegacyValidParameters
* @dataProvider getValidParameters
*/
public function testLegacyRouteParameters(string $parameter, $value, string $getter)
public function testLegacyRouteParameters(string $methodName, string $getter, $expectedReturn)
{
$this->expectDeprecation('Since symfony/routing 5.3: Passing an array as first argument to "Symfony\Component\Routing\Annotation\Route::__construct" is deprecated. Use named arguments instead.');

$route = new Route([$parameter => $value]);
$this->assertEquals($route->$getter(), $value);
$route = $this->getMethodAnnotation($methodName, false);
$this->assertEquals($route->$getter(), $expectedReturn);
}

public function getValidParameters(): iterable
{
return [
['requirements', ['locale' => 'en'], 'getRequirements'],
['options', ['compiler_class' => 'RouteCompiler'], 'getOptions'],
['name', 'blog_index', 'getName'],
['defaults', ['_controller' => 'MyBlogBundle:Blog:index'], 'getDefaults'],
['schemes', ['https'], 'getSchemes'],
['methods', ['GET', 'POST'], 'getMethods'],
['host', '{locale}.example.com', 'getHost'],
['condition', 'context.getMethod() == "GET"', 'getCondition'],
['simplePath', 'getPath', '/Blog'],
['localized', 'getLocalizedPaths', ['nl' => '/hier', 'en' => '/here']],
['requirements', 'getRequirements', ['locale' => 'en']],
['options', 'getOptions', ['compiler_class' => 'RouteCompiler']],
['name', 'getName', 'blog_index'],
['defaults', 'getDefaults', ['_controller' => 'MyBlogBundle:Blog:index']],
['schemes', 'getSchemes', ['https']],
['methods', 'getMethods', ['GET', 'POST']],
['host', 'getHost', '{locale}.example.com'],
['condition', 'getCondition', 'context.getMethod() == \'GET\''],
];
}

public function getLegacyValidParameters(): iterable
{
yield from $this->getValidParameters();

yield ['value', '/Blog', 'getPath'];
yield ['value', ['nl' => '/hier', 'en' => '/here'], 'getLocalizedPaths'];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
<?php

namespace Symfony\Component\Routing\Tests\Fixtures\AnnotationFixtures;

use Symfony\Component\Routing\Annotation\Route;

class FooController
{
/**
* @Route("/Blog")
*/
public function simplePath()
{
}

/**
* @Route({"nl":"/hier","en":"/here"})
*/
public function localized()
{
}

/**
* @Route(requirements={"locale":"en"})
*/
public function requirements()
{
}

/**
* @Route(options={"compiler_class":"RouteCompiler"})
*/
public function options()
{
}

/**
* @Route(name="blog_index")
*/
public function name()
{
}

/**
* @Route(defaults={"_controller":"MyBlogBundle:Blog:index"})
*/
public function defaults()
{
}

/**
* @Route(schemes={"https"})
*/
public function schemes()
{
}

/**
* @Route(methods={"GET","POST"})
*/
public function methods()
{
}

/**
* @Route(host="{locale}.example.com")
*/
public function host()
{
}

/**
* @Route(condition="context.getMethod() == 'GET'")
*/
public function condition()
{
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

namespace Symfony\Component\Routing\Tests\Fixtures\AttributeFixtures;

use Symfony\Component\Routing\Annotation\Route;

class FooController
{
#[Route('/Blog')]
public function simplePath()
{
}

#[Route(['nl' => '/hier', 'en' => '/here'])]
public function localized()
{
}

#[Route(requirements: ['locale' => 'en'])]
public function requirements()
{
}

#[Route(options: ['compiler_class' => 'RouteCompiler'])]
public function options()
{
}

#[Route(name: 'blog_index')]
public function name()
{
}

#[Route(defaults: ['_controller' => 'MyBlogBundle:Blog:index'])]
public function defaults()
{
}

#[Route(schemes: ['https'])]
public function schemes()
{
}

#[Route(methods: ['GET', 'POST'])]
public function methods()
{
}

#[Route(host: '{locale}.example.com')]
public function host()
{
}

#[Route(condition: 'context.getMethod() == \'GET\'')]
public function condition()
{
}
}