From d771e449ec92958d7cf7c88b3c8092d3e46c611e Mon Sep 17 00:00:00 2001 From: Robin Chalas Date: Thu, 25 Feb 2021 17:07:22 +0100 Subject: [PATCH] [HttpKernel] Handle multi-attribute controller arguments --- UPGRADE-5.3.md | 2 + UPGRADE-6.0.md | 2 + .../Attribute/ArgumentInterface.php | 4 ++ src/Symfony/Component/HttpKernel/CHANGELOG.md | 3 ++ .../ControllerMetadata/ArgumentMetadata.php | 52 +++++++++++++++++-- .../ArgumentMetadataFactory.php | 24 ++------- .../ArgumentMetadataFactoryTest.php | 10 ++-- .../ArgumentMetadataTest.php | 28 ++++++++++ .../Tests/Fixtures/Attribute/Foo.php | 2 +- .../Controller/AttributeController.php | 2 +- .../Security/Http/Attribute/CurrentUser.php | 4 +- .../Http/Controller/UserValueResolver.php | 2 +- .../Controller/UserValueResolverTest.php | 5 +- .../Component/Security/Http/composer.json | 2 +- 14 files changed, 103 insertions(+), 39 deletions(-) diff --git a/UPGRADE-5.3.md b/UPGRADE-5.3.md index bdb61f495be61..6444adefd8c59 100644 --- a/UPGRADE-5.3.md +++ b/UPGRADE-5.3.md @@ -44,6 +44,8 @@ HttpFoundation HttpKernel ---------- + * Deprecate `ArgumentInterface` + * Deprecate `ArgumentMetadata::getAttribute()`, use `getAttributes()` instead * Marked the class `Symfony\Component\HttpKernel\EventListener\DebugHandlersListener` as internal Messenger diff --git a/UPGRADE-6.0.md b/UPGRADE-6.0.md index 5d93fe244800b..a254493d4e0f6 100644 --- a/UPGRADE-6.0.md +++ b/UPGRADE-6.0.md @@ -92,6 +92,8 @@ HttpFoundation HttpKernel ---------- + * Remove `ArgumentInterface` + * Remove `ArgumentMetadata::getAttribute()`, use `getAttributes()` instead * Made `WarmableInterface::warmUp()` return a list of classes or files to preload on PHP 7.4+ * Removed support for `service:action` syntax to reference controllers. Use `serviceOrFqcn::method` instead. diff --git a/src/Symfony/Component/HttpKernel/Attribute/ArgumentInterface.php b/src/Symfony/Component/HttpKernel/Attribute/ArgumentInterface.php index 8f0c6fb8b060c..78769f1ac0bd2 100644 --- a/src/Symfony/Component/HttpKernel/Attribute/ArgumentInterface.php +++ b/src/Symfony/Component/HttpKernel/Attribute/ArgumentInterface.php @@ -11,8 +11,12 @@ namespace Symfony\Component\HttpKernel\Attribute; +trigger_deprecation('symfony/http-kernel', '5.3', 'The "%s" interface is deprecated.', ArgumentInterface::class); + /** * Marker interface for controller argument attributes. + * + * @deprecated since Symfony 5.3 */ interface ArgumentInterface { diff --git a/src/Symfony/Component/HttpKernel/CHANGELOG.md b/src/Symfony/Component/HttpKernel/CHANGELOG.md index e20b6c881d2d1..e2509ba1bbeca 100644 --- a/src/Symfony/Component/HttpKernel/CHANGELOG.md +++ b/src/Symfony/Component/HttpKernel/CHANGELOG.md @@ -4,6 +4,9 @@ CHANGELOG 5.3 --- + * Deprecate `ArgumentInterface` + * Add `ArgumentMetadata::getAttributes()` + * Deprecate `ArgumentMetadata::getAttribute()`, use `getAttributes()` instead * marked the class `Symfony\Component\HttpKernel\EventListener\DebugHandlersListener` as internal 5.2.0 diff --git a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php index 3454ff6e49417..1a9ebc0c3a5d1 100644 --- a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php +++ b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadata.php @@ -20,15 +20,20 @@ */ class ArgumentMetadata { + public const IS_INSTANCEOF = 2; + private $name; private $type; private $isVariadic; private $hasDefaultValue; private $defaultValue; private $isNullable; - private $attribute; + private $attributes; - public function __construct(string $name, ?string $type, bool $isVariadic, bool $hasDefaultValue, $defaultValue, bool $isNullable = false, ?ArgumentInterface $attribute = null) + /** + * @param object[] $attributes + */ + public function __construct(string $name, ?string $type, bool $isVariadic, bool $hasDefaultValue, $defaultValue, bool $isNullable = false, $attributes = []) { $this->name = $name; $this->type = $type; @@ -36,7 +41,13 @@ public function __construct(string $name, ?string $type, bool $isVariadic, bool $this->hasDefaultValue = $hasDefaultValue; $this->defaultValue = $defaultValue; $this->isNullable = $isNullable || null === $type || ($hasDefaultValue && null === $defaultValue); - $this->attribute = $attribute; + + if (null === $attributes || $attributes instanceof ArgumentInterface) { + trigger_deprecation('symfony/http-kernel', '5.3', 'The "%s" constructor expects an array of PHP attributes as last argument, %s given.', __CLASS__, get_debug_type($attributes)); + $attributes = $attributes ? [$attributes] : []; + } + + $this->attributes = $attributes; } /** @@ -114,6 +125,39 @@ public function getDefaultValue() */ public function getAttribute(): ?ArgumentInterface { - return $this->attribute; + trigger_deprecation('symfony/http-kernel', '5.3', 'Method "%s()" is deprecated, use "getAttributes()" instead.', __METHOD__); + + if (!$this->attributes) { + return null; + } + + return $this->attributes[0] instanceof ArgumentInterface ? $this->attributes[0] : null; + } + + /** + * @return object[] + */ + public function getAttributes(string $name = null, int $flags = 0): array + { + if (!$name) { + return $this->attributes; + } + + $attributes = []; + if ($flags & self::IS_INSTANCEOF) { + foreach ($this->attributes as $attribute) { + if ($attribute instanceof $name) { + $attributes[] = $attribute; + } + } + } else { + foreach ($this->attributes as $attribute) { + if (\get_class($attribute) === $name) { + $attributes[] = $attribute; + } + } + } + + return $attributes; } } diff --git a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php index f53bf065b96b1..a2feb05c10340 100644 --- a/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php +++ b/src/Symfony/Component/HttpKernel/ControllerMetadata/ArgumentMetadataFactory.php @@ -11,9 +11,6 @@ namespace Symfony\Component\HttpKernel\ControllerMetadata; -use Symfony\Component\HttpKernel\Attribute\ArgumentInterface; -use Symfony\Component\HttpKernel\Exception\InvalidMetadataException; - /** * Builds {@see ArgumentMetadata} objects based on the given Controller. * @@ -37,28 +34,15 @@ public function createArgumentMetadata($controller): array } foreach ($reflection->getParameters() as $param) { - $attribute = null; if (\PHP_VERSION_ID >= 80000) { - $reflectionAttributes = $param->getAttributes(ArgumentInterface::class, \ReflectionAttribute::IS_INSTANCEOF); - - if (\count($reflectionAttributes) > 1) { - $representative = $controller; - - if (\is_array($representative)) { - $representative = sprintf('%s::%s()', \get_class($representative[0]), $representative[1]); - } elseif (\is_object($representative)) { - $representative = \get_class($representative); + foreach ($param->getAttributes() as $reflectionAttribute) { + if (class_exists($reflectionAttribute->getName())) { + $attributes[] = $reflectionAttribute->newInstance(); } - - throw new InvalidMetadataException(sprintf('Controller "%s" has more than one attribute for "$%s" argument.', $representative, $param->getName())); - } - - if (isset($reflectionAttributes[0])) { - $attribute = $reflectionAttributes[0]->newInstance(); } } - $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param, $reflection), $param->isVariadic(), $param->isDefaultValueAvailable(), $param->isDefaultValueAvailable() ? $param->getDefaultValue() : null, $param->allowsNull(), $attribute); + $arguments[] = new ArgumentMetadata($param->getName(), $this->getType($param, $reflection), $param->isVariadic(), $param->isDefaultValueAvailable(), $param->isDefaultValueAvailable() ? $param->getDefaultValue() : null, $param->allowsNull(), $attributes ?? []); } return $arguments; diff --git a/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php b/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php index 3c57c3161c8c5..d952e424c557e 100644 --- a/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataFactoryTest.php @@ -15,7 +15,6 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadataFactory; -use Symfony\Component\HttpKernel\Exception\InvalidMetadataException; use Symfony\Component\HttpKernel\Tests\Fixtures\Attribute\Foo; use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\AttributeController; use Symfony\Component\HttpKernel\Tests\Fixtures\Controller\BasicTypesController; @@ -128,18 +127,17 @@ public function testAttributeSignature() $arguments = $this->factory->createArgumentMetadata([new AttributeController(), 'action']); $this->assertEquals([ - new ArgumentMetadata('baz', 'string', false, false, null, false, new Foo('bar')), + new ArgumentMetadata('baz', 'string', false, false, null, false, [new Foo('bar')]), ], $arguments); } /** * @requires PHP 8 */ - public function testAttributeSignatureError() + public function testMultipleAttributes() { - $this->expectException(InvalidMetadataException::class); - - $this->factory->createArgumentMetadata([new AttributeController(), 'invalidAction']); + $this->factory->createArgumentMetadata([new AttributeController(), 'multiAttributeArg']); + $this->assertCount(1, $this->factory->createArgumentMetadata([new AttributeController(), 'multiAttributeArg'])[0]->getAttributes()); } private function signature1(self $foo, array $bar, callable $baz) diff --git a/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataTest.php b/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataTest.php index fef6cd00025f0..45b15e174445f 100644 --- a/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataTest.php +++ b/src/Symfony/Component/HttpKernel/Tests/ControllerMetadata/ArgumentMetadataTest.php @@ -12,10 +12,15 @@ namespace Symfony\Component\HttpKernel\Tests\ControllerMetadata; use PHPUnit\Framework\TestCase; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; +use Symfony\Component\HttpKernel\Attribute\ArgumentInterface; use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata; +use Symfony\Component\HttpKernel\Tests\Fixtures\Attribute\Foo; class ArgumentMetadataTest extends TestCase { + use ExpectDeprecationTrait; + public function testWithBcLayerWithDefault() { $argument = new ArgumentMetadata('foo', 'string', false, true, 'default value'); @@ -41,4 +46,27 @@ public function testDefaultValueUnavailable() $this->assertFalse($argument->hasDefaultValue()); $argument->getDefaultValue(); } + + /** + * @group legacy + */ + public function testLegacyAttribute() + { + $attribute = $this->createMock(ArgumentInterface::class); + + $this->expectDeprecation('Since symfony/http-kernel 5.3: The "Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata" constructor expects an array of PHP attributes as last argument, %s given.'); + $this->expectDeprecation('Since symfony/http-kernel 5.3: Method "Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata::getAttribute()" is deprecated, use "getAttributes()" instead.'); + + $argument = new ArgumentMetadata('foo', 'string', false, true, 'default value', true, $attribute); + $this->assertSame($attribute, $argument->getAttribute()); + } + + /** + * @requires PHP 8 + */ + public function testGetAttributes() + { + $argument = new ArgumentMetadata('foo', 'string', false, true, 'default value', true, [new Foo('bar')]); + $this->assertEquals([new Foo('bar')], $argument->getAttributes()); + } } diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Attribute/Foo.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Attribute/Foo.php index 96a03adaad0bd..e01a5a6e8ddcd 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Attribute/Foo.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Attribute/Foo.php @@ -14,7 +14,7 @@ use Symfony\Component\HttpKernel\Attribute\ArgumentInterface; #[\Attribute(\Attribute::TARGET_PARAMETER)] -class Foo implements ArgumentInterface +class Foo { private $foo; diff --git a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AttributeController.php b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AttributeController.php index 910f418ae1eb1..d6e0cde58d883 100644 --- a/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AttributeController.php +++ b/src/Symfony/Component/HttpKernel/Tests/Fixtures/Controller/AttributeController.php @@ -18,6 +18,6 @@ class AttributeController public function action(#[Foo('bar')] string $baz) { } - public function invalidAction(#[Foo('bar'), Foo('bar')] string $baz) { + public function multiAttributeArg(#[Foo('bar'), Undefined('bar')] string $baz) { } } diff --git a/src/Symfony/Component/Security/Http/Attribute/CurrentUser.php b/src/Symfony/Component/Security/Http/Attribute/CurrentUser.php index e9202ed2b35db..413f982ecc5ac 100644 --- a/src/Symfony/Component/Security/Http/Attribute/CurrentUser.php +++ b/src/Symfony/Component/Security/Http/Attribute/CurrentUser.php @@ -11,12 +11,10 @@ namespace Symfony\Component\Security\Http\Attribute; -use Symfony\Component\HttpKernel\Attribute\ArgumentInterface; - /** * Indicates that a controller argument should receive the current logged user. */ #[\Attribute(\Attribute::TARGET_PARAMETER)] -class CurrentUser implements ArgumentInterface +class CurrentUser { } diff --git a/src/Symfony/Component/Security/Http/Controller/UserValueResolver.php b/src/Symfony/Component/Security/Http/Controller/UserValueResolver.php index 4b469edf8b10e..715004318e18c 100644 --- a/src/Symfony/Component/Security/Http/Controller/UserValueResolver.php +++ b/src/Symfony/Component/Security/Http/Controller/UserValueResolver.php @@ -37,7 +37,7 @@ public function supports(Request $request, ArgumentMetadata $argument): bool { // with the attribute, the type can be any UserInterface implementation // otherwise, the type must be UserInterface - if (UserInterface::class !== $argument->getType() && !$argument->getAttribute() instanceof CurrentUser) { + if (UserInterface::class !== $argument->getType() && !$argument->getAttributes(CurrentUser::class, ArgumentMetadata::IS_INSTANCEOF)) { return false; } diff --git a/src/Symfony/Component/Security/Http/Tests/Controller/UserValueResolverTest.php b/src/Symfony/Component/Security/Http/Tests/Controller/UserValueResolverTest.php index ca3197e5e4f9a..bfded5d20a141 100644 --- a/src/Symfony/Component/Security/Http/Tests/Controller/UserValueResolverTest.php +++ b/src/Symfony/Component/Security/Http/Tests/Controller/UserValueResolverTest.php @@ -77,7 +77,8 @@ public function testResolveWithAttribute() $tokenStorage->setToken($token); $resolver = new UserValueResolver($tokenStorage); - $metadata = new ArgumentMetadata('foo', null, false, false, null, false, new CurrentUser()); + $metadata = $this->createMock(ArgumentMetadata::class); + $metadata = new ArgumentMetadata('foo', null, false, false, null, false, [new CurrentUser()]); $this->assertTrue($resolver->supports(Request::create('/'), $metadata)); $this->assertSame([$user], iterator_to_array($resolver->resolve(Request::create('/'), $metadata))); @@ -89,7 +90,7 @@ public function testResolveWithAttributeAndNoUser() $tokenStorage->setToken(new UsernamePasswordToken('username', 'password', 'provider')); $resolver = new UserValueResolver($tokenStorage); - $metadata = new ArgumentMetadata('foo', null, false, false, null, false, new CurrentUser()); + $metadata = new ArgumentMetadata('foo', null, false, false, null, false, [new CurrentUser()]); $this->assertFalse($resolver->supports(Request::create('/'), $metadata)); } diff --git a/src/Symfony/Component/Security/Http/composer.json b/src/Symfony/Component/Security/Http/composer.json index c2297fc0535b4..d953ac23a63a3 100644 --- a/src/Symfony/Component/Security/Http/composer.json +++ b/src/Symfony/Component/Security/Http/composer.json @@ -20,7 +20,7 @@ "symfony/deprecation-contracts": "^2.1", "symfony/security-core": "^5.3", "symfony/http-foundation": "^5.2", - "symfony/http-kernel": "^5.2", + "symfony/http-kernel": "^5.3", "symfony/polyfill-php80": "^1.15", "symfony/property-access": "^4.4|^5.0" },