From a34487d867d0d61c5ade3be8e9037cf005e77b25 Mon Sep 17 00:00:00 2001 From: Beno!t POLASZEK Date: Fri, 4 Oct 2019 15:28:49 +0200 Subject: [PATCH 1/5] Add defaults configuration --- .../ApiPlatformExtension.php | 32 ++++++++++++ .../DependencyInjection/Configuration.php | 2 + .../Resources/config/metadata/annotation.xml | 1 + .../Bundle/Resources/config/metadata/yaml.xml | 1 + .../AnnotationResourceMetadataFactory.php | 21 +++++--- .../ExtractorResourceMetadataFactory.php | 11 +++- .../ApiPlatformExtensionTest.php | 6 +++ .../AnnotationResourceMetadataFactoryTest.php | 32 ++++++++++++ .../ExtractorResourceMetadataFactoryTest.php | 50 +++++++++++++++++++ 9 files changed, 148 insertions(+), 8 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index cf39090b833..e6f4fa3a36f 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection; +use ApiPlatform\Core\Annotation\ApiResource; use ApiPlatform\Core\Api\FilterInterface; use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationCollectionExtensionInterface; use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationItemExtensionInterface; @@ -191,6 +192,37 @@ private function registerCommonConfiguration(ContainerBuilder $container, array if ($config['name_converter']) { $container->setAlias('api_platform.name_converter', $config['name_converter']); } + $container->setParameter('api_platform.defaults', $this->normalizeDefaults($config['defaults'] ?? [])); + } + + private function normalizeDefaults(array $defaults): array + { + $normalizedDefaults = ['attributes' => []]; + $reflClass = new \ReflectionClass(ApiResource::class); + + foreach ($defaults as $key => $value) { + // camelCase allows us comparing to @ApiResource properties + $camelCased = lcfirst( + preg_replace_callback( + '/(^|_|\.)+(.)/', + static function ($match) { + return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); + }, + $key + ) + ); + + // snake_case is the reference for all attribute keys + $snakeCased = strtolower(preg_replace('/[A-Z]/', '_\\0', $key)); + + if (($reflProperty = $reflClass->getProperty($camelCased)) && $reflProperty->isPublic()) { + $normalizedDefaults[$snakeCased] = $value; + } else { + $normalizedDefaults['attributes'][$snakeCased] = $value; + } + } + + return $normalizedDefaults; } private function registerMetadataConfiguration(ContainerBuilder $container, array $config, XmlFileLoader $loader): void diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index b49c43b28ed..be83304060d 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -154,6 +154,8 @@ public function getConfigTreeBuilder() ->arrayNode('resource_class_directories') ->prototype('scalar')->end() ->end() + ->variableNode('defaults') + ->end() ->end(); $this->addDoctrineOrmSection($rootNode); diff --git a/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml b/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml index 70556d4f8bc..6440d8c921e 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/metadata/annotation.xml @@ -14,6 +14,7 @@ + %api_platform.defaults% diff --git a/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml b/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml index 1657eb99b86..7344a04a22e 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/metadata/yaml.xml @@ -18,6 +18,7 @@ + %api_platform.defaults% diff --git a/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php b/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php index d941e0e4918..22b6b6370d0 100644 --- a/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php +++ b/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php @@ -27,11 +27,16 @@ final class AnnotationResourceMetadataFactory implements ResourceMetadataFactory { private $reader; private $decorated; + /** + * @var array + */ + private $defaults; - public function __construct(Reader $reader, ResourceMetadataFactoryInterface $decorated = null) + public function __construct(Reader $reader, ResourceMetadataFactoryInterface $decorated = null, array $defaults = []) { $this->reader = $reader; $this->decorated = $decorated; + $this->defaults = $defaults + ['attributes' => []]; } /** @@ -78,16 +83,18 @@ private function handleNotFound(?ResourceMetadata $parentPropertyMetadata, strin private function createMetadata(ApiResource $annotation, ResourceMetadata $parentResourceMetadata = null): ResourceMetadata { + $attributes = (null === $annotation->attributes && [] === $this->defaults['attributes']) ? null : (array) $annotation->attributes + $this->defaults['attributes']; + if (!$parentResourceMetadata) { return new ResourceMetadata( $annotation->shortName, - $annotation->description, - $annotation->iri, - $annotation->itemOperations, - $annotation->collectionOperations, - $annotation->attributes, + $annotation->description ?? $this->defaults['description'] ?? null, + $annotation->iri ?? $this->defaults['iri'] ?? null, + $annotation->itemOperations ?? $this->defaults['item_operations'] ?? null, + $annotation->collectionOperations ?? $this->defaults['collection_operations'] ?? null, + $attributes, $annotation->subresourceOperations, - $annotation->graphql + $annotation->graphql ?? $this->defaults['graphql'] ?? null ); } diff --git a/src/Metadata/Resource/Factory/ExtractorResourceMetadataFactory.php b/src/Metadata/Resource/Factory/ExtractorResourceMetadataFactory.php index 369380b4364..fee663cf97c 100644 --- a/src/Metadata/Resource/Factory/ExtractorResourceMetadataFactory.php +++ b/src/Metadata/Resource/Factory/ExtractorResourceMetadataFactory.php @@ -27,11 +27,13 @@ final class ExtractorResourceMetadataFactory implements ResourceMetadataFactoryI { private $extractor; private $decorated; + private $defaults; - public function __construct(ExtractorInterface $extractor, ResourceMetadataFactoryInterface $decorated = null) + public function __construct(ExtractorInterface $extractor, ResourceMetadataFactoryInterface $decorated = null, array $defaults = []) { $this->extractor = $extractor; $this->decorated = $decorated; + $this->defaults = $defaults + ['attributes' => []]; } /** @@ -52,6 +54,13 @@ public function create(string $resourceClass): ResourceMetadata return $this->handleNotFound($parentResourceMetadata, $resourceClass); } + $resource['description'] = $resource['description'] ?? $this->defaults['description'] ?? null; + $resource['iri'] = $resource['iri'] ?? $this->defaults['iri'] ?? null; + $resource['itemOperations'] = $resource['itemOperations'] ?? $this->defaults['item_operations'] ?? null; + $resource['collectionOperations'] = $resource['collectionOperations'] ?? $this->defaults['collection_operations'] ?? null; + $resource['graphql'] = $resource['graphql'] ?? $this->defaults['graphql'] ?? null; + $resource['attributes'] = (null === $resource['attributes'] && [] === $this->defaults['attributes']) ? null : (array) $resource['attributes'] + $this->defaults['attributes']; + return $this->update($parentResourceMetadata ?: new ResourceMetadata(), $resource); } diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 11f23d41b3f..9ea0aa8466b 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -144,6 +144,9 @@ class ApiPlatformExtensionTest extends TestCase 'doctrine_mongodb_odm' => [ 'enabled' => false, ], + 'defaults' => [ + 'attributes' => [], + ], ]]; private $extension; @@ -694,6 +697,7 @@ public function testEnableElasticsearch() $containerBuilderProphecy->registerForAutoconfiguration(RequestBodySearchCollectionExtensionInterface::class)->willReturn($this->childDefinitionProphecy)->shouldBeCalled(); $containerBuilderProphecy->setParameter('api_platform.elasticsearch.hosts', ['http://elasticsearch:9200'])->shouldBeCalled(); $containerBuilderProphecy->setParameter('api_platform.elasticsearch.mapping', [])->shouldBeCalled(); + $containerBuilderProphecy->setParameter('api_platform.defaults', ['attributes' => []])->shouldBeCalled(); $config = self::DEFAULT_CONFIG; $config['api_platform']['elasticsearch'] = [ @@ -804,6 +808,7 @@ private function getPartialContainerBuilderProphecy() 'api_platform.http_cache.shared_max_age' => null, 'api_platform.http_cache.vary' => ['Accept'], 'api_platform.http_cache.public' => null, + 'api_platform.defaults' => ['attributes' => []], 'api_platform.enable_entrypoint' => true, 'api_platform.enable_docs' => true, ]; @@ -1076,6 +1081,7 @@ private function getBaseContainerBuilderProphecy(array $doctrineIntegrationsToLo 'api_platform.resource_class_directories' => Argument::type('array'), 'api_platform.validator.serialize_payload_fields' => [], 'api_platform.elasticsearch.enabled' => false, + 'api_platform.defaults' => ['attributes' => []], ]; foreach ($parameters as $key => $value) { diff --git a/tests/Metadata/Resource/Factory/AnnotationResourceMetadataFactoryTest.php b/tests/Metadata/Resource/Factory/AnnotationResourceMetadataFactoryTest.php index 27835cd7468..744e3e66053 100644 --- a/tests/Metadata/Resource/Factory/AnnotationResourceMetadataFactoryTest.php +++ b/tests/Metadata/Resource/Factory/AnnotationResourceMetadataFactoryTest.php @@ -46,6 +46,38 @@ public function testCreate($reader, $decorated, string $expectedShortName, strin $this->assertEquals(['foo' => 'bar'], $metadata->getGraphql()); } + public function testCreateWithDefaults() + { + $defaults = [ + 'shortName' => 'Default shortname should not be ignored', + 'description' => 'CHANGEME!', + 'collection_operations' => ['get'], + 'item_operations' => ['get', 'put'], + 'attributes' => [ + 'pagination_items_per_page' => 4, + 'pagination_maximum_items_per_page' => 6, + ], + ]; + + $annotation = new ApiResource([ + 'itemOperations' => ['get', 'delete'], + 'attributes' => [ + 'pagination_maximum_items_per_page' => 10, + ], + ]); + $reader = $this->prophesize(Reader::class); + $reader->getClassAnnotation(Argument::type(\ReflectionClass::class), ApiResource::class)->willReturn($annotation)->shouldBeCalled(); + $factory = new AnnotationResourceMetadataFactory($reader->reveal(), null, $defaults); + $metadata = $factory->create(Dummy::class); + + $this->assertNull($metadata->getShortName()); + $this->assertEquals('CHANGEME!', $metadata->getDescription()); + $this->assertEquals(['get'], $metadata->getCollectionOperations()); + $this->assertEquals(['get', 'delete'], $metadata->getItemOperations()); + $this->assertEquals(4, $metadata->getAttribute('pagination_items_per_page')); + $this->assertEquals(10, $metadata->getAttribute('pagination_maximum_items_per_page')); + } + public function testCreateWithoutAttributes() { $annotation = new ApiResource([]); diff --git a/tests/Metadata/Resource/Factory/ExtractorResourceMetadataFactoryTest.php b/tests/Metadata/Resource/Factory/ExtractorResourceMetadataFactoryTest.php index 4b16a9cafb2..28aea8d4b52 100644 --- a/tests/Metadata/Resource/Factory/ExtractorResourceMetadataFactoryTest.php +++ b/tests/Metadata/Resource/Factory/ExtractorResourceMetadataFactoryTest.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Exception\ResourceClassNotFoundException; +use ApiPlatform\Core\Metadata\Extractor\ExtractorInterface; use ApiPlatform\Core\Metadata\Extractor\XmlExtractor; use ApiPlatform\Core\Metadata\Extractor\YamlExtractor; use ApiPlatform\Core\Metadata\Resource\Factory\ExtractorResourceMetadataFactory; @@ -23,6 +24,7 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ShortNameResourceMetadataFactory; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Tests\Fixtures\DummyResourceInterface; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy; /** @@ -286,4 +288,52 @@ public function testItSupportsInterfaceAsAResource() $resourceMetadata = $shortNameResourceMetadataFactory->create(DummyResourceInterface::class); $this->assertSame('DummyResourceInterface', $resourceMetadata->getShortName()); } + + public function testItFallbacksToDefaultConfiguration() + { + $defaults = [ + 'shortName' => 'Default shortname should not be ignored', + 'description' => 'CHANGEME!', + 'collection_operations' => ['get'], + 'item_operations' => ['get', 'put'], + 'attributes' => [ + 'pagination_items_per_page' => 4, + 'pagination_maximum_items_per_page' => 6, + ], + ]; + $resourceConfiguration = [ + Dummy::class => [ + 'shortName' => null, + 'description' => null, + 'subresourceOperations' => null, + 'itemOperations' => ['get', 'delete'], + 'attributes' => [ + 'pagination_maximum_items_per_page' => 10, + ], + ], + ]; + + $extractor = new class($resourceConfiguration) implements ExtractorInterface { + private $resources; + + public function __construct(array $resources) + { + $this->resources = $resources; + } + + public function getResources(): array + { + return $this->resources; + } + }; + $factory = new ExtractorResourceMetadataFactory($extractor, null, $defaults); + $metadata = $factory->create(Dummy::class); + + $this->assertNull($metadata->getShortName()); + $this->assertEquals('CHANGEME!', $metadata->getDescription()); + $this->assertEquals(['get'], $metadata->getCollectionOperations()); + $this->assertEquals(['get', 'delete'], $metadata->getItemOperations()); + $this->assertEquals(4, $metadata->getAttribute('pagination_items_per_page')); + $this->assertEquals(10, $metadata->getAttribute('pagination_maximum_items_per_page')); + } } From 67c80a631f86688e0e134764fb081d4d7cf0cf3f Mon Sep 17 00:00:00 2001 From: Beno!t POLASZEK Date: Fri, 4 Oct 2019 16:13:57 +0200 Subject: [PATCH 2/5] Suggest defaults in configuration --- .../ApiPlatformExtension.php | 11 +++++--- .../DependencyInjection/Configuration.php | 26 +++++++++++++++++-- 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index e6f4fa3a36f..7b372dac888 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -215,9 +215,14 @@ static function ($match) { // snake_case is the reference for all attribute keys $snakeCased = strtolower(preg_replace('/[A-Z]/', '_\\0', $key)); - if (($reflProperty = $reflClass->getProperty($camelCased)) && $reflProperty->isPublic()) { - $normalizedDefaults[$snakeCased] = $value; - } else { + try { + $reflProperty = $reflClass->getProperty($camelCased); + if ($reflProperty->isPublic()) { + $normalizedDefaults[$snakeCased] = $value; + } else { + $normalizedDefaults['attributes'][$snakeCased] = $value; + } + } catch (\ReflectionException $e) { $normalizedDefaults['attributes'][$snakeCased] = $value; } } diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index be83304060d..4eb9b4671d4 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection; +use ApiPlatform\Core\Annotation\ApiResource; use ApiPlatform\Core\Bridge\Elasticsearch\Metadata\Document\DocumentMetadata; use ApiPlatform\Core\Exception\FilterValidationException; use ApiPlatform\Core\Exception\InvalidArgumentException; @@ -154,8 +155,6 @@ public function getConfigTreeBuilder() ->arrayNode('resource_class_directories') ->prototype('scalar')->end() ->end() - ->variableNode('defaults') - ->end() ->end(); $this->addDoctrineOrmSection($rootNode); @@ -181,6 +180,8 @@ public function getConfigTreeBuilder() 'jsonld' => ['mime_types' => ['application/ld+json']], ]); + $this->addDefaultsSection($rootNode); + return $treeBuilder; } @@ -496,4 +497,25 @@ private function addFormatSection(ArrayNodeDefinition $rootNode, string $key, ar ->end() ->end(); } + + private function addDefaultsSection(ArrayNodeDefinition $rootNode): void + { + $defaultsNode = $rootNode->children()->arrayNode('defaults')->ignoreExtraKeys(); + $properties = (new \ReflectionClass(ApiResource::class))->getProperties(); + $ignoredOptions = [ + 'shortName', + 'attributes', + 'subresourceOperations', + ]; + + foreach ($properties as $property) { + $optionName = $property->getName(); + if (\in_array($optionName, $ignoredOptions, true)) { // "attributes" is a sub-level and not a config option + continue; + } + $snakeCased = strtolower(preg_replace('/[A-Z]/', '_\\0', $optionName)); + + $defaultsNode->children()->variableNode($snakeCased); + } + } } From ce19c2c880750e27a9a79879ddb5f90ad09650c7 Mon Sep 17 00:00:00 2001 From: Beno!t POLASZEK Date: Mon, 7 Oct 2019 10:11:31 +0200 Subject: [PATCH 3/5] Apply changes from code review --- .../ApiPlatformExtension.php | 23 +++++++------------ .../DependencyInjection/Configuration.php | 14 ++++++----- .../AnnotationResourceMetadataFactory.php | 3 --- 3 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index 7b372dac888..0205c8c8409 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -49,6 +49,7 @@ use Symfony\Component\Finder\Finder; use Symfony\Component\HttpClient\HttpClientTrait; use Symfony\Component\HttpKernel\DependencyInjection\Extension; +use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter; use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Yaml\Yaml; @@ -198,26 +199,18 @@ private function registerCommonConfiguration(ContainerBuilder $container, array private function normalizeDefaults(array $defaults): array { $normalizedDefaults = ['attributes' => []]; - $reflClass = new \ReflectionClass(ApiResource::class); + $reflectionClass = new \ReflectionClass(ApiResource::class); + $nameConverter = new CamelCaseToSnakeCaseNameConverter(); - foreach ($defaults as $key => $value) { + foreach ($defaults as $option => $value) { // camelCase allows us comparing to @ApiResource properties - $camelCased = lcfirst( - preg_replace_callback( - '/(^|_|\.)+(.)/', - static function ($match) { - return ('.' === $match[1] ? '_' : '').strtoupper($match[2]); - }, - $key - ) - ); - // snake_case is the reference for all attribute keys - $snakeCased = strtolower(preg_replace('/[A-Z]/', '_\\0', $key)); + $camelCased = $nameConverter->denormalize($option); + $snakeCased = $nameConverter->normalize($option); try { - $reflProperty = $reflClass->getProperty($camelCased); - if ($reflProperty->isPublic()) { + $reflectionProperty = $reflectionClass->getProperty($camelCased); + if ($reflectionProperty->isPublic()) { $normalizedDefaults[$snakeCased] = $value; } else { $normalizedDefaults['attributes'][$snakeCased] = $value; diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index 4eb9b4671d4..91edb00b70e 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -34,6 +34,7 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Messenger\MessageBusInterface; use Symfony\Component\Serializer\Exception\ExceptionInterface as SerializerExceptionInterface; +use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter; /** * The configuration of the bundle. @@ -500,20 +501,21 @@ private function addFormatSection(ArrayNodeDefinition $rootNode, string $key, ar private function addDefaultsSection(ArrayNodeDefinition $rootNode): void { + $nameConverter = new CamelCaseToSnakeCaseNameConverter(); $defaultsNode = $rootNode->children()->arrayNode('defaults')->ignoreExtraKeys(); $properties = (new \ReflectionClass(ApiResource::class))->getProperties(); $ignoredOptions = [ - 'shortName', - 'attributes', - 'subresourceOperations', + 'shortName', // It doesn't make sense for multiple resources to share the same name. + 'subresourceOperations', // Same here. + 'attributes', // The "attributes" option is a sub-level and not a config option. ]; foreach ($properties as $property) { - $optionName = $property->getName(); - if (\in_array($optionName, $ignoredOptions, true)) { // "attributes" is a sub-level and not a config option + $option = $property->getName(); + if (\in_array($option, $ignoredOptions, true)) { continue; } - $snakeCased = strtolower(preg_replace('/[A-Z]/', '_\\0', $optionName)); + $snakeCased = $nameConverter->normalize($option); $defaultsNode->children()->variableNode($snakeCased); } diff --git a/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php b/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php index 22b6b6370d0..c04364b5b9b 100644 --- a/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php +++ b/src/Metadata/Resource/Factory/AnnotationResourceMetadataFactory.php @@ -27,9 +27,6 @@ final class AnnotationResourceMetadataFactory implements ResourceMetadataFactory { private $reader; private $decorated; - /** - * @var array - */ private $defaults; public function __construct(Reader $reader, ResourceMetadataFactoryInterface $decorated = null, array $defaults = []) From 6ff4855ba16a4dbf21615c16a0b3fd719f2c4527 Mon Sep 17 00:00:00 2001 From: Beno!t POLASZEK Date: Mon, 7 Oct 2019 11:35:13 +0200 Subject: [PATCH 4/5] Add deprecation notices for legacy defaults --- .../DependencyInjection/Configuration.php | 62 ++++++++++++++++--- 1 file changed, 52 insertions(+), 10 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index 91edb00b70e..cfa6d781ddc 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -130,13 +130,41 @@ public function getConfigTreeBuilder() ->canBeDisabled() ->addDefaultsIfNotSet() ->children() - ->booleanNode('enabled')->defaultTrue()->info('To enable or disable pagination for all resource collections by default.')->end() - ->booleanNode('partial')->defaultFalse()->info('To enable or disable partial pagination for all resource collections by default when pagination is enabled.')->end() - ->booleanNode('client_enabled')->defaultFalse()->info('To allow the client to enable or disable the pagination.')->end() - ->booleanNode('client_items_per_page')->defaultFalse()->info('To allow the client to set the number of items per page.')->end() - ->booleanNode('client_partial')->defaultFalse()->info('To allow the client to enable or disable partial pagination.')->end() - ->integerNode('items_per_page')->defaultValue(30)->info('The default number of items per page.')->end() - ->integerNode('maximum_items_per_page')->defaultNull()->info('The maximum number of items per page.')->end() + ->booleanNode('enabled') + ->setDeprecated('The use of the `collection.pagination.enabled` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.pagination_enabled` instead.') + ->defaultTrue() + ->info('To enable or disable pagination for all resource collections by default.') + ->end() + ->booleanNode('partial') + ->setDeprecated('The use of the `collection.pagination.partial` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.pagination_partial` instead.') + ->defaultFalse() + ->info('To enable or disable partial pagination for all resource collections by default when pagination is enabled.') + ->end() + ->booleanNode('client_enabled') + ->setDeprecated('The use of the `collection.pagination.client_enabled` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.pagination_client_enabled` instead.') + ->defaultFalse() + ->info('To allow the client to enable or disable the pagination.') + ->end() + ->booleanNode('client_items_per_page') + ->setDeprecated('The use of the `collection.pagination.client_items_per_page` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.pagination_client_items_per_page` instead.') + ->defaultFalse() + ->info('To allow the client to set the number of items per page.') + ->end() + ->booleanNode('client_partial') + ->setDeprecated('The use of the `collection.pagination.client_partial` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.pagination_client_partial` instead.') + ->defaultFalse() + ->info('To allow the client to enable or disable partial pagination.') + ->end() + ->integerNode('items_per_page') + ->setDeprecated('The use of the `collection.pagination.items_per_page` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.pagination_items_per_page` instead.') + ->defaultValue(30) + ->info('The default number of items per page.') + ->end() + ->integerNode('maximum_items_per_page') + ->setDeprecated('The use of the `collection.pagination.maximum_items_per_page` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.pagination_maximum_items_per_page` instead.') + ->defaultNull() + ->info('The maximum number of items per page.') + ->end() ->scalarNode('page_parameter_name')->defaultValue('page')->cannotBeEmpty()->info('The default name of the parameter handling the page number.')->end() ->scalarNode('enabled_parameter_name')->defaultValue('pagination')->cannotBeEmpty()->info('The name of the query parameter to enable or disable pagination.')->end() ->scalarNode('items_per_page_parameter_name')->defaultValue('itemsPerPage')->cannotBeEmpty()->info('The name of the query parameter to set the number of items per page.')->end() @@ -315,16 +343,30 @@ private function addHttpCacheSection(ArrayNodeDefinition $rootNode): void ->arrayNode('http_cache') ->addDefaultsIfNotSet() ->children() - ->booleanNode('etag')->defaultTrue()->info('Automatically generate etags for API responses.')->end() - ->integerNode('max_age')->defaultNull()->info('Default value for the response max age.')->end() - ->integerNode('shared_max_age')->defaultNull()->info('Default value for the response shared (proxy) max age.')->end() + ->booleanNode('etag') + ->setDeprecated('The use of the `http_cache.etag` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.cache_headers.etag` instead.') + ->defaultTrue() + ->info('Automatically generate etags for API responses.') + ->end() + ->integerNode('max_age') + ->setDeprecated('The use of the `http_cache.max_age` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.cache_headers.max_age` instead.') + ->defaultNull() + ->info('Default value for the response max age.') + ->end() + ->integerNode('shared_max_age') + ->setDeprecated('The use of the `http_cache.shared_max_age` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.cache_headers.shared_max_age` instead.') + ->defaultNull() + ->info('Default value for the response shared (proxy) max age.') + ->end() ->arrayNode('vary') + ->setDeprecated('The use of the `http_cache.vary` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.cache_headers.vary` instead.') ->defaultValue(['Accept']) ->prototype('scalar')->end() ->info('Default values of the "Vary" HTTP header.') ->end() ->booleanNode('public')->defaultNull()->info('To make all responses public by default.')->end() ->arrayNode('invalidation') + ->setDeprecated('The use of the `http_cache.invalidation` has been deprecated in 2.6 and will be removed in 3.0. Use `defaults.cache_headers.invalidation` instead.') ->info('Enable the tags-based cache invalidation system.') ->canBeEnabled() ->children() From 4af78723180cb38722353c842dadf34057e693a7 Mon Sep 17 00:00:00 2001 From: Beno!t POLASZEK Date: Tue, 8 Oct 2019 10:51:17 +0200 Subject: [PATCH 5/5] Apply @dunglas' suggestions --- src/Annotation/ApiResource.php | 49 +++++++++++++++++++ .../ApiPlatformExtension.php | 29 +++++------ .../DependencyInjection/Configuration.php | 30 ++++++------ 3 files changed, 76 insertions(+), 32 deletions(-) diff --git a/src/Annotation/ApiResource.php b/src/Annotation/ApiResource.php index 5f2a6ac1cf9..27a392564f9 100644 --- a/src/Annotation/ApiResource.php +++ b/src/Annotation/ApiResource.php @@ -73,6 +73,55 @@ final class ApiResource { use AttributesHydratorTrait; + /** + * @internal + * + * @see \ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection\Configuration::addDefaultsSection + */ + public const CONFIGURABLE_DEFAULTS = [ + 'accessControl', + 'accessControlMessage', + 'security', + 'securityMessage', + 'securityPostDenormalize', + 'securityPostDenormalizeMessage', + 'cacheHeaders', + 'collectionOperations', + 'denormalizationContext', + 'deprecationReason', + 'description', + 'elasticsearch', + 'fetchPartial', + 'forceEager', + 'formats', + 'filters', + 'graphql', + 'hydraContext', + 'input', + 'iri', + 'itemOperations', + 'mercure', + 'messenger', + 'normalizationContext', + 'openapiContext', + 'order', + 'output', + 'paginationClientEnabled', + 'paginationClientItemsPerPage', + 'paginationClientPartial', + 'paginationEnabled', + 'paginationFetchJoinCollection', + 'paginationItemsPerPage', + 'maximumItemsPerPage', + 'paginationMaximumItemsPerPage', + 'paginationPartial', + 'paginationViaCursor', + 'routePrefix', + 'sunset', + 'swaggerContext', + 'validationGroups', + ]; + /** * @var string */ diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index 0205c8c8409..d0fcbaaa300 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -13,7 +13,6 @@ namespace ApiPlatform\Core\Bridge\Symfony\Bundle\DependencyInjection; -use ApiPlatform\Core\Annotation\ApiResource; use ApiPlatform\Core\Api\FilterInterface; use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationCollectionExtensionInterface; use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationItemExtensionInterface; @@ -49,7 +48,6 @@ use Symfony\Component\Finder\Finder; use Symfony\Component\HttpClient\HttpClientTrait; use Symfony\Component\HttpKernel\DependencyInjection\Extension; -use Symfony\Component\Serializer\NameConverter\CamelCaseToSnakeCaseNameConverter; use Symfony\Component\Validator\Validator\ValidatorInterface; use Symfony\Component\Yaml\Yaml; @@ -199,24 +197,19 @@ private function registerCommonConfiguration(ContainerBuilder $container, array private function normalizeDefaults(array $defaults): array { $normalizedDefaults = ['attributes' => []]; - $reflectionClass = new \ReflectionClass(ApiResource::class); - $nameConverter = new CamelCaseToSnakeCaseNameConverter(); + $rootLevelOptions = [ + 'description', + 'iri', + 'item_operations', + 'collection_operations', + 'graphql', + ]; foreach ($defaults as $option => $value) { - // camelCase allows us comparing to @ApiResource properties - // snake_case is the reference for all attribute keys - $camelCased = $nameConverter->denormalize($option); - $snakeCased = $nameConverter->normalize($option); - - try { - $reflectionProperty = $reflectionClass->getProperty($camelCased); - if ($reflectionProperty->isPublic()) { - $normalizedDefaults[$snakeCased] = $value; - } else { - $normalizedDefaults['attributes'][$snakeCased] = $value; - } - } catch (\ReflectionException $e) { - $normalizedDefaults['attributes'][$snakeCased] = $value; + if (\in_array($option, $rootLevelOptions, true)) { + $normalizedDefaults[$option] = $value; + } else { + $normalizedDefaults['attributes'][$option] = $value; } } diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index cfa6d781ddc..ca8a5b0d4f0 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -544,21 +544,23 @@ private function addFormatSection(ArrayNodeDefinition $rootNode, string $key, ar private function addDefaultsSection(ArrayNodeDefinition $rootNode): void { $nameConverter = new CamelCaseToSnakeCaseNameConverter(); - $defaultsNode = $rootNode->children()->arrayNode('defaults')->ignoreExtraKeys(); - $properties = (new \ReflectionClass(ApiResource::class))->getProperties(); - $ignoredOptions = [ - 'shortName', // It doesn't make sense for multiple resources to share the same name. - 'subresourceOperations', // Same here. - 'attributes', // The "attributes" option is a sub-level and not a config option. - ]; - - foreach ($properties as $property) { - $option = $property->getName(); - if (\in_array($option, $ignoredOptions, true)) { - continue; - } - $snakeCased = $nameConverter->normalize($option); + $defaultsNode = $rootNode->children()->arrayNode('defaults'); + $defaultsNode + ->ignoreExtraKeys() + ->beforeNormalization() + ->always(function (array $defaults) use ($nameConverter) { + $normalizedDefaults = []; + foreach ($defaults as $option => $value) { + $option = $nameConverter->normalize($option); + $normalizedDefaults[$option] = $value; + } + + return $normalizedDefaults; + }); + + foreach (ApiResource::CONFIGURABLE_DEFAULTS as $attribute) { + $snakeCased = $nameConverter->normalize($attribute); $defaultsNode->children()->variableNode($snakeCased); } }