From 2b2d468f06a63ecfa4928d5d631953acb624c181 Mon Sep 17 00:00:00 2001 From: Vincent Chalamon Date: Wed, 16 Nov 2022 16:56:47 +0100 Subject: [PATCH] fix(metadata): operations must inherit from resource and defaults --- ...actorResourceMetadataCollectionFactory.php | 74 +++++++++++++++++-- .../Resources/config/metadata/resource.xml | 1 + .../Bundle/Resources/config/metadata/yaml.xml | 1 + .../ResourceMetadataCompatibilityTest.php | 74 ++++++++++++++++++- 4 files changed, 141 insertions(+), 9 deletions(-) diff --git a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php index 93740809c60..ca5b46bf571 100644 --- a/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php +++ b/src/Metadata/Resource/Factory/ExtractorResourceMetadataCollectionFactory.php @@ -26,6 +26,9 @@ use ApiPlatform\Metadata\Put; use ApiPlatform\Metadata\Resource\DeprecationMetadataTrait; use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; +use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter; /** * Creates a resource metadata from {@see Resource} extractors (XML, YAML). @@ -38,12 +41,14 @@ final class ExtractorResourceMetadataCollectionFactory implements ResourceMetada private $extractor; private $decorated; private $defaults; + private $logger; - public function __construct(ResourceExtractorInterface $extractor, ResourceMetadataCollectionFactoryInterface $decorated = null, array $defaults = []) + public function __construct(ResourceExtractorInterface $extractor, ResourceMetadataCollectionFactoryInterface $decorated = null, array $defaults = [], LoggerInterface $logger = null) { $this->extractor = $extractor; $this->decorated = $decorated; $this->defaults = $defaults; + $this->logger = $logger ?? new NullLogger(); } /** @@ -182,13 +187,7 @@ private function buildGraphQlOperations(?array $data, ApiResource $resource): ar private function getOperationWithDefaults(ApiResource $resource, HttpOperation $operation): HttpOperation { - foreach (($this->defaults['attributes'] ?? []) as $key => $value) { - [$key, $value] = $this->getKeyValue($key, $value); - if (null === $operation->{'get'.ucfirst($key)}()) { - $operation = $operation->{'with'.ucfirst($key)}($value); - } - } - + // Inherit from resource defaults foreach (get_class_methods($resource) as $methodName) { if (0 !== strpos($methodName, 'get')) { continue; @@ -205,6 +204,65 @@ private function getOperationWithDefaults(ApiResource $resource, HttpOperation $ $operation = $operation->{'with'.substr($methodName, 3)}($value); } + $operation = $operation->withExtraProperties(array_merge( + $resource->getExtraProperties(), + $operation->getExtraProperties() + )); + + // Add global defaults attributes to the operation + $operation = $this->addGlobalDefaults($operation); + + if ($operation->getRouteName()) { + /** @var HttpOperation $operation */ + $operation = $operation->withName($operation->getRouteName()); + } + + // Check for name conflict + if ($operation->getName() && null !== ($operations = $resource->getOperations())) { + if (!$operations->has($operation->getName())) { + return $operation; + } + + $this->logger->warning(sprintf('The operation "%s" already exists on the resource "%s", pick a different name or leave it empty. In the meantime we will generate a unique name.', $operation->getName(), $resource->getClass())); + /** @var HttpOperation $operation */ + $operation = $operation->withName(''); + } + return $operation; } + + private function addGlobalDefaults(HttpOperation $operation): HttpOperation + { + if (!$this->camelCaseToSnakeCaseNameConverter) { + $this->camelCaseToSnakeCaseNameConverter = new CamelCaseToSnakeCaseNameConverter(); + } + + $extraProperties = []; + foreach ($this->defaults as $key => $value) { + $upperKey = ucfirst($this->camelCaseToSnakeCaseNameConverter->denormalize($key)); + $getter = 'get'.$upperKey; + + if (!method_exists($operation, $getter)) { + if (!isset($extraProperties[$key])) { + $extraProperties[$key] = $value; + } + + continue; + } + + $currentValue = $operation->{$getter}(); + + if (\is_array($currentValue) && $currentValue) { + $operation = $operation->{'with'.$upperKey}(array_merge($value, $currentValue)); + } + + if (null !== $currentValue) { + continue; + } + + $operation = $operation->{'with'.$upperKey}($value); + } + + return $operation->withExtraProperties(array_merge($extraProperties, $operation->getExtraProperties())); + } } diff --git a/src/Symfony/Bundle/Resources/config/metadata/resource.xml b/src/Symfony/Bundle/Resources/config/metadata/resource.xml index 3e011de0d62..333037d623f 100644 --- a/src/Symfony/Bundle/Resources/config/metadata/resource.xml +++ b/src/Symfony/Bundle/Resources/config/metadata/resource.xml @@ -21,6 +21,7 @@ %api_platform.defaults% + diff --git a/src/Symfony/Bundle/Resources/config/metadata/yaml.xml b/src/Symfony/Bundle/Resources/config/metadata/yaml.xml index 2e417e7c578..f3d0209f3f3 100644 --- a/src/Symfony/Bundle/Resources/config/metadata/yaml.xml +++ b/src/Symfony/Bundle/Resources/config/metadata/yaml.xml @@ -24,6 +24,7 @@ %api_platform.defaults% + diff --git a/tests/Metadata/Extractor/ResourceMetadataCompatibilityTest.php b/tests/Metadata/Extractor/ResourceMetadataCompatibilityTest.php index b340b05543a..fbe7ef5069f 100644 --- a/tests/Metadata/Extractor/ResourceMetadataCompatibilityTest.php +++ b/tests/Metadata/Extractor/ResourceMetadataCompatibilityTest.php @@ -37,6 +37,7 @@ use ApiPlatform\Tests\Metadata\Extractor\Adapter\YamlResourceAdapter; use PHPUnit\Framework\AssertionFailedError; use PHPUnit\Framework\TestCase; +use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter; /** * Ensures XML and YAML mappings are fully compatible with ApiResource. @@ -49,6 +50,9 @@ final class ResourceMetadataCompatibilityTest extends TestCase private const RESOURCE_CLASS = Comment::class; private const SHORT_NAME = 'Comment'; + private const DEFAULTS = [ + 'route_prefix' => '/v1', + ]; private const FIXTURES = [ null, [ @@ -208,6 +212,10 @@ final class ResourceMetadataCompatibilityTest extends TestCase 'priority' => 200, 'extraProperties' => [ 'foo' => 'bar', + 'custom_property' => 'Lorem ipsum dolor sit amet', + 'another_custom_property' => [ + 'Lorem ipsum' => 'Dolor sit amet', + ], ], ], ], @@ -329,6 +337,10 @@ final class ResourceMetadataCompatibilityTest extends TestCase 'priority' => 200, 'extraProperties' => [ 'foo' => 'bar', + 'custom_property' => 'Lorem ipsum dolor sit amet', + 'another_custom_property' => [ + 'Lorem ipsum' => 'Dolor sit amet', + ], ], ], [ @@ -420,7 +432,7 @@ public function testValidMetadata(string $extractorClass, ResourceAdapterInterfa try { $extractor = new $extractorClass($adapter(self::RESOURCE_CLASS, $parameters, self::FIXTURES)); - $factory = new ExtractorResourceMetadataCollectionFactory($extractor); + $factory = new ExtractorResourceMetadataCollectionFactory($extractor, null, self::DEFAULTS); $collection = $factory->create(self::RESOURCE_CLASS); } catch (\Exception $exception) { throw new AssertionFailedError('Failed asserting that the schema is valid according to '.ApiResource::class, 0, $exception); @@ -595,6 +607,7 @@ private function withGraphQlOperations(array $values, ?array $fixtures): array private function getOperationWithDefaults(ApiResource $resource, HttpOperation $operation): HttpOperation { + // Inherit from resource defaults foreach (get_class_methods($resource) as $methodName) { if (0 !== strpos($methodName, 'get')) { continue; @@ -611,6 +624,65 @@ private function getOperationWithDefaults(ApiResource $resource, HttpOperation $ $operation = $operation->{'with'.substr($methodName, 3)}($value); } + $operation = $operation->withExtraProperties(array_merge( + $resource->getExtraProperties(), + $operation->getExtraProperties() + )); + + // Add global defaults attributes to the operation + $operation = $this->addGlobalDefaults($operation); + + if ($operation->getRouteName()) { + /** @var HttpOperation $operation */ + $operation = $operation->withName($operation->getRouteName()); + } + + // Check for name conflict + if ($operation->getName() && null !== ($operations = $resource->getOperations())) { + if (!$operations->has($operation->getName())) { + return $operation; + } + + /** @var HttpOperation $operation */ + $operation = $operation->withName(''); + } + return $operation; } + + private function addGlobalDefaults(HttpOperation $operation): HttpOperation + { + if (!$this->camelCaseToSnakeCaseNameConverter) { + $this->camelCaseToSnakeCaseNameConverter = new CamelCaseToSnakeCaseNameConverter(); + } + + $extraProperties = []; + foreach (self::DEFAULTS as $key => $value) { + $upperKey = ucfirst($this->camelCaseToSnakeCaseNameConverter->denormalize($key)); + $getter = 'get'.$upperKey; + + if (!method_exists($operation, $getter)) { + if (!isset($extraProperties[$key])) { + $extraProperties[$key] = $value; + } + + continue; + } + + $currentValue = $operation->{$getter}(); + + /* @phpstan-ignore-next-line */ + if (\is_array($currentValue) && $currentValue && \is_array($value) && $value) { + $operation = $operation->{'with'.$upperKey}(array_merge($value, $currentValue)); + } + + if (null !== $currentValue) { + continue; + } + + $operation = $operation->{'with'.$upperKey}($value); + } + + return $operation->withExtraProperties(array_merge($extraProperties, $operation->getExtraProperties())); + } }