From 98f4b8fcab39b1d09110cc133206e27782ea4791 Mon Sep 17 00:00:00 2001 From: Vincent <407859+vincentchalamon@users.noreply.github.com> Date: Thu, 7 Mar 2024 19:06:23 +0100 Subject: [PATCH] fix(metadata): Operations priority sort (#6206) * fix(metadata): fix Operations sort * test: priority test --------- Co-authored-by: soyuka --- src/Metadata/ApiResource.php | 1 + src/Metadata/Operation.php | 6 ++++ ...butesResourceMetadataCollectionFactory.php | 6 ++-- ...ollerResourceMetadataCollectionFactory.php | 2 +- ...nNameResourceMetadataCollectionFactory.php | 2 +- ...plateResourceMetadataCollectionFactory.php | 2 +- src/Metadata/Tests/OperationsTest.php | 36 +++++++++++++++++++ .../ApiResource/OperationPriorities.php | 33 +++++++++++++++++ tests/Symfony/Bundle/Test/ApiTestCaseTest.php | 10 ++++++ 9 files changed, 93 insertions(+), 5 deletions(-) create mode 100644 src/Metadata/Tests/OperationsTest.php create mode 100644 tests/Fixtures/TestBundle/ApiResource/OperationPriorities.php diff --git a/src/Metadata/ApiResource.php b/src/Metadata/ApiResource.php index 1f8a1836fc6..c2c74e65797 100644 --- a/src/Metadata/ApiResource.php +++ b/src/Metadata/ApiResource.php @@ -1018,6 +1018,7 @@ public function withOperations(Operations $operations): self { $self = clone $this; $self->operations = $operations; + $self->operations->sort(); return $self; } diff --git a/src/Metadata/Operation.php b/src/Metadata/Operation.php index d1f56e98da1..c6b933563aa 100644 --- a/src/Metadata/Operation.php +++ b/src/Metadata/Operation.php @@ -794,6 +794,12 @@ public function __construct( protected ?bool $serialize = null, protected ?bool $fetchPartial = null, protected ?bool $forceEager = null, + /** + * The priority helps with the order of operations when looping over a resource's operations. + * It can be usefull when we loop over operations to find a matching IRI, although most of the use cases + * should be covered by the HttpOperation::itemUriTemplate or the ApiProperty::uriTemplate functionalities. + * Sort is ascendant: a lower priority comes first in the list. + */ protected ?int $priority = null, protected ?string $name = null, protected $provider = null, diff --git a/src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php index 969de3a4a67..cd391d0cb1a 100644 --- a/src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php @@ -119,9 +119,11 @@ private function buildResourceOperations(array $attributes, string $resourceClas } [$key, $operation] = $this->getOperationWithDefaults($resources[$index], $operationAttribute); - $operation = $operation->withPriority(++$operationPriority); + if (null === $operation->getPriority()) { + $operation = $operation->withPriority(++$operationPriority); + } $operations = $resources[$index]->getOperations() ?? new Operations(); - $resources[$index] = $resources[$index]->withOperations($operations->add($key, $operation)->sort()); + $resources[$index] = $resources[$index]->withOperations($operations->add($key, $operation)); } // Loop again and set default operations if none where found diff --git a/src/Metadata/Resource/Factory/MainControllerResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/MainControllerResourceMetadataCollectionFactory.php index 970f4ca1014..7987032f836 100644 --- a/src/Metadata/Resource/Factory/MainControllerResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/MainControllerResourceMetadataCollectionFactory.php @@ -52,7 +52,7 @@ public function create(string $resourceClass): ResourceMetadataCollection } } - $resource = $resource->withOperations($operations->sort()); + $resource = $resource->withOperations($operations); $resourceMetadataCollection[$i] = $resource; } diff --git a/src/Metadata/Resource/Factory/OperationNameResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/OperationNameResourceMetadataCollectionFactory.php index 9127deaec85..f79c092c254 100644 --- a/src/Metadata/Resource/Factory/OperationNameResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/OperationNameResourceMetadataCollectionFactory.php @@ -56,7 +56,7 @@ public function create(string $resourceClass): ResourceMetadataCollection $operations->remove($operationName)->add($newOperationName, $operation->withName($newOperationName)); } - $resourceMetadataCollection[$i] = $resource->withOperations($operations->sort()); + $resourceMetadataCollection[$i] = $resource->withOperations($operations); } return $resourceMetadataCollection; diff --git a/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php index 56708711e8e..f75e3ef2962 100644 --- a/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/UriTemplateResourceMetadataCollectionFactory.php @@ -88,7 +88,7 @@ public function create(string $resourceClass): ResourceMetadataCollection $operations->add($operation->getName(), $operation); } - $resource = $resource->withOperations($operations->sort()); + $resource = $resource->withOperations($operations); $resourceMetadataCollection[$i] = $resource; } diff --git a/src/Metadata/Tests/OperationsTest.php b/src/Metadata/Tests/OperationsTest.php new file mode 100644 index 00000000000..74cf8b3e199 --- /dev/null +++ b/src/Metadata/Tests/OperationsTest.php @@ -0,0 +1,36 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Metadata\Tests; + +use ApiPlatform\Metadata\Get; +use ApiPlatform\Metadata\Operations; +use PHPUnit\Framework\TestCase; + +final class OperationsTest extends TestCase +{ + public function testOperationsHaveNameIfNotSet(): void + { + $operations = new Operations([new Get(name: 'a'), new Get(name: 'b')]); + + foreach ($operations as $name => $operation) { + $this->assertEquals($name, $operation->getName()); + } + } + + public function testOperationAreSorted(): void + { + $operations = new Operations(['a' => new Get(priority: 0), 'b' => new Get(priority: -1)]); + $this->assertEquals(['b', 'a'], array_keys(iterator_to_array($operations))); + } +} diff --git a/tests/Fixtures/TestBundle/ApiResource/OperationPriorities.php b/tests/Fixtures/TestBundle/ApiResource/OperationPriorities.php new file mode 100644 index 00000000000..0d032e63f38 --- /dev/null +++ b/tests/Fixtures/TestBundle/ApiResource/OperationPriorities.php @@ -0,0 +1,33 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Tests\Fixtures\TestBundle\ApiResource; + +use ApiPlatform\Metadata\Get; + +#[Get(name: 'a', priority: 1, uriTemplate: 'operation_priority/{id}', provider: [self::class, 'shouldNotBeCalled'])] +#[Get(name: 'b', priority: -1, uriTemplate: 'operation_priority/{id}', provider: [self::class, 'shouldBeCalled'])] +class OperationPriorities +{ + public int $id = 1; + + public static function shouldBeCalled(): self + { + return new self(); + } + + public static function shouldNotBeCalled(): self + { + throw new \Exception('fail'); + } +} diff --git a/tests/Symfony/Bundle/Test/ApiTestCaseTest.php b/tests/Symfony/Bundle/Test/ApiTestCaseTest.php index b236a1d03de..86f40e65a38 100644 --- a/tests/Symfony/Bundle/Test/ApiTestCaseTest.php +++ b/tests/Symfony/Bundle/Test/ApiTestCaseTest.php @@ -267,6 +267,16 @@ public function testFindIriBy(): void $this->assertNull(self::findIriBy($resource, ['name' => 'not-exist'])); } + public function testGetPrioritizedOperation(): void + { + $r = self::createClient()->request('GET', '/operation_priority/1', [ + 'headers' => [ + 'accept' => 'application/ld+json', + ], + ]); + $this->assertResponseIsSuccessful(); + } + /** * @group mercure */