fix(metadata): defaults with new metadata allows extra keys (#4743)

* fix(metadata): defaults with new metadata allows extra keys

* fix older versions

* fix older versions

* fix older versions

* fix deprecation

Author: soyuka

src/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactory.php

@@ -234,16 +234,22 @@ private function getOperationWithDefaults(ApiResource $resource, Operation $oper
      */
     private function addGlobalDefaults($operation)
     {
+        $extraProperties = $operation->getExtraProperties();
         foreach ($this->defaults['attributes'] as $key => $value) {
-            [$key, $value] = $this->getKeyValue($key, $value);
-            $upperKey = ucfirst($key);
+            [$newKey, $value] = $this->getKeyValue($key, $value);
+            $upperKey = ucfirst($newKey);
             $getter = 'get'.$upperKey;
-            if (method_exists($operation, $getter) && null === $operation->{$getter}()) {
+
+            if (!method_exists($operation, $getter)) {
+                if (!isset($extraProperties[$key])) {
+                    $extraProperties[$key] = $value;
+                }
+            } elseif (null === $operation->{$getter}()) {
                 $operation = $operation->{'with'.$upperKey}($value);
             }
         }
 
-        return $operation;
+        return $operation->withExtraProperties($extraProperties);
     }
 
     private function getResourceWithDefaults(string $resourceClass, string $shortName, ApiResource $resource): ApiResource

src/Metadata/WithResourceTrait.php

@@ -39,6 +39,6 @@ protected function copyFrom($resource)
             }
         }
 
-        return $self;
+        return $self->withExtraProperties(array_merge($resource->getExtraProperties(), $self->getExtraProperties()));
     }
 }

src/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

@@ -253,7 +253,7 @@ private function registerCommonConfiguration(ContainerBuilder $container, array
             $container->setAlias('api_platform.name_converter', $config['name_converter']);
         }
         $container->setParameter('api_platform.asset_package', $config['asset_package']);
-        $container->setParameter('api_platform.defaults', $this->normalizeDefaults($config['defaults'] ?? []));
+        $container->setParameter('api_platform.defaults', $this->normalizeDefaults($config['defaults'] ?? [], $config['metadata_backward_compatibility_layer'] ?? false));
     }
 
     /**
@@ -274,12 +274,21 @@ private function getPaginationDefaults(array $defaults, array $collectionPaginat
         return array_merge($collectionPaginationConfiguration, $paginationOptions);
     }
 
-    private function normalizeDefaults(array $defaults): array
+    private function normalizeDefaults(array $defaults, bool $compatibility = false): array
     {
         $normalizedDefaults = ['attributes' => $defaults['attributes'] ?? []];
         unset($defaults['attributes']);
 
-        [$publicProperties,] = ApiResource::getConfigMetadata();
+        $publicProperties = [];
+
+        if ($compatibility) {
+            [$publicProperties,] = ApiResource::getConfigMetadata();
+        } else {
+            $rc = new \ReflectionClass(ApiResource::class);
+            foreach ($rc->getConstructor()->getParameters() as $param) {
+                $publicProperties[$param->getName()] = true;
+            }
+        }
 
         $nameConverter = new CamelCaseToSnakeCaseNameConverter();
         foreach ($defaults as $option => $value) {

src/Symfony/Bundle/DependencyInjection/Configuration.php

@@ -631,7 +631,7 @@ private function addDefaultsSection(ArrayNodeDefinition $rootNode): void
         $defaultsNode = $rootNode->children()->arrayNode('defaults');
 
         $defaultsNode
-            ->ignoreExtraKeys()
+            ->ignoreExtraKeys(false)
             ->beforeNormalization()
             ->always(static function (array $defaults) use ($nameConverter) {
                 $normalizedDefaults = [];

tests/Core/Bridge/Symfony/Bundle/Command/SwaggerCommandUnitTest.php

@@ -18,13 +18,15 @@
 use ApiPlatform\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
 use ApiPlatform\Metadata\Resource\ResourceNameCollection;
 use PHPUnit\Framework\MockObject\MockObject;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
 use Symfony\Component\Console\Input\ArrayInput;
 use Symfony\Component\Console\Output\BufferedOutput;
 use Symfony\Component\Serializer\Normalizer\NormalizerInterface;
 
 class SwaggerCommandUnitTest extends KernelTestCase
 {
+    use ExpectDeprecationTrait;
     /** @var MockObject&NormalizerInterface */
     private $normalizer;
 
@@ -50,8 +52,12 @@ protected function setUp(): void
             ->willReturn(new ResourceNameCollection());
     }
 
+    /**
+     * @group legacy
+     */
     public function testDocumentationJsonDoesNotUseEscapedSlashes(): void
     {
+        $this->expectDeprecation('The command "api:swagger:export" is using pre-2.7 metadata, new metadata will not appear, use "api:openapi:export" instead.');
         $this->normalizer->method('normalize')
             ->with(self::isInstanceOf(Documentation::class))
             ->willReturn(['a-jsonable-documentation' => 'containing/some/slashes']);

tests/Metadata/Resource/Factory/AttributesResourceMetadataCollectionFactoryTest.php

@@ -139,7 +139,8 @@ public function testCreateWithDefaults(): void
     {
         $attributeResourceMetadataCollectionFactory = new AttributesResourceMetadataCollectionFactory(null, null, ['attributes' => ['cache_headers' => ['max_age' => 60], 'non_existing_attribute' => 'foo']]);
 
-        $operation = new HttpOperation(shortName: 'AttributeDefaultOperations', class: AttributeDefaultOperations::class, cacheHeaders: ['max_age' => 60], paginationItemsPerPage: 10);
+        $operation = new HttpOperation(shortName: 'AttributeDefaultOperations', class: AttributeDefaultOperations::class, cacheHeaders: ['max_age' => 60], paginationItemsPerPage: 10, extraProperties: ['non_existing_attribute' => 'foo']);
+
         $this->assertEquals(new ResourceMetadataCollection(AttributeDefaultOperations::class, [
             new ApiResource(
                 shortName: 'AttributeDefaultOperations',
@@ -154,7 +155,8 @@ class: AttributeDefaultOperations::class,
                     '_api_AttributeDefaultOperations_delete' => (new Delete())->withOperation($operation),
                 ],
                 cacheHeaders: ['max_age' => 60],
-                paginationItemsPerPage: 10
+                paginationItemsPerPage: 10,
+                extraProperties: ['non_existing_attribute' => 'foo']
             ),
         ]), $attributeResourceMetadataCollectionFactory->create(AttributeDefaultOperations::class));
     }

tests/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php

@@ -53,6 +53,7 @@
 use Doctrine\ORM\OptimisticLockException;
 use phpDocumentor\Reflection\DocBlockFactoryInterface;
 use PHPUnit\Framework\TestCase;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Bundle\SecurityBundle\SecurityBundle;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
@@ -63,6 +64,7 @@
 
 class ApiPlatformExtensionTest extends TestCase
 {
+    use ExpectDeprecationTrait;
     use ProphecyTrait;
 
     public const DEFAULT_CONFIG = ['api_platform' => [
@@ -111,16 +113,9 @@ class ApiPlatformExtensionTest extends TestCase
             'order_parameter_name' => 'order',
             'order_nulls_comparison' => null,
             'pagination' => [
-                'client_enabled' => false,
-                'client_items_per_page' => false,
-                'enabled' => true,
+                'page_parameter_name' => 'page',
                 'enabled_parameter_name' => 'pagination',
-                'items_per_page' => 30,
                 'items_per_page_parameter_name' => 'itemsPerPage',
-                'maximum_items_per_page' => 30,
-                'page_parameter_name' => 'page',
-                'partial' => false,
-                'client_partial' => false,
                 'partial_parameter_name' => 'partial',
             ],
         ],
@@ -136,8 +131,6 @@ class ApiPlatformExtensionTest extends TestCase
             OptimisticLockException::class => Response::HTTP_CONFLICT,
         ],
         'show_webby' => true,
-        // TODO: to remove in 3.0
-        'allow_plain_identifiers' => false,
         'eager_loading' => [
             'enabled' => true,
             'max_joins' => 30,
@@ -1036,6 +1029,55 @@ public function testGraphQlConfigurationMetadataBackwardCompatibilityLayer(): vo
         $this->assertServiceHasTags('api_platform.graphql.command.export_command', ['console.command']);
     }
 
+    /**
+     * @group legacy
+     */
+    public function testLegacyPlainIdentifier()
+    {
+        // There's an issue with deprecations being different on lower versions
+        if (\PHP_VERSION_ID < 80000) {
+            $this->markTestSkipped();
+        }
+
+        $config = self::DEFAULT_CONFIG;
+        $config['api_platform']['allow_plain_identifiers'] = false;
+        $this->expectDeprecation('Since api-platform/core 2.7: The use of `allow_plain_identifiers` has been deprecated in 2.7 and will be removed in 3.0.');
+        (new ApiPlatformExtension())->load($config, $this->container);
+    }
+
+    /**
+     * @group legacy
+     */
+    public function testLegacyPaginationCollectionOptions()
+    {
+        // There's an issue with deprecations being different on lower versions
+        if (\PHP_VERSION_ID < 80000) {
+            $this->markTestSkipped();
+        }
+
+        $config = self::DEFAULT_CONFIG;
+
+        $config['api_platform']['collection']['pagination'] = [
+            'client_enabled' => false,
+            'client_items_per_page' => false,
+            'enabled' => true,
+            'items_per_page' => 30,
+            'maximum_items_per_page' => 30,
+            'partial' => false,
+            'client_partial' => false,
+        ];
+
+        $this->expectDeprecation('Since api-platform/core 2.6: 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.');
+        $this->expectDeprecation('Since api-platform/core 2.6: 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.');
+        $this->expectDeprecation('Since api-platform/core 2.6: 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.');
+        $this->expectDeprecation('Since api-platform/core 2.6: 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.');
+        $this->expectDeprecation('Since api-platform/core 2.6: 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.');
+        $this->expectDeprecation('Since api-platform/core 2.6: 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.');
+        $this->expectDeprecation('Since api-platform/core 2.6: 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.');
+
+        (new ApiPlatformExtension())->load($config, $this->container);
+    }
+
     public function testLegacyBundlesConfigurationFosUserBundle(): void
     {
         $config = self::DEFAULT_CONFIG;
@@ -1824,4 +1866,17 @@ public function testAutoConfigurableInterfaces(): void
 
         $this->assertEmpty(array_diff(array_keys($interfaces), $has), 'Not all expected interfaces are autoconfigurable.');
     }
+
+    public function testDefaults()
+    {
+        $config = self::DEFAULT_CONFIG;
+        $config['api_platform']['defaults'] = [
+            'something' => 'test',
+            'attributes' => ['else' => 'foo'],
+        ];
+
+        (new ApiPlatformExtension())->load($config, $this->container);
+
+        $this->assertEquals($this->container->getParameter('api_platform.defaults'), ['attributes' => ['else' => 'foo', 'something' => 'test']]);
+    }
 }