Merge pull request #1803 from soyuka/merge-2.2

soyuka · web-flow · commit 4727a876aa02 · 2018-04-03T12:09:10.000+02:00
Merge 2.2
diff --git a/features/graphql/mutation.feature b/features/graphql/mutation.feature
@@ -221,6 +221,45 @@ Feature: GraphQL mutation support
     And the JSON node "data.updateCompositeRelation.value" should be equal to "Modified value."
     And the JSON node "data.updateCompositeRelation.clientMutationId" should be equal to "myId"
 
+  Scenario: Create an item with a custom UUID
+    When I send the following GraphQL request:
+    """
+    mutation {
+      createWritableId(input: {_id: "c6b722fe-0331-48c4-a214-f81f9f1ca082", name: "Foo", clientMutationId: "m"}) {
+        id
+        _id
+        name
+        clientMutationId
+      }
+    }
+    """
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/json"
+    And the JSON node "data.createWritableId.id" should be equal to "/writable_ids/c6b722fe-0331-48c4-a214-f81f9f1ca082"
+    And the JSON node "data.createWritableId._id" should be equal to "c6b722fe-0331-48c4-a214-f81f9f1ca082"
+    And the JSON node "data.createWritableId.name" should be equal to "Foo"
+    And the JSON node "data.createWritableId.clientMutationId" should be equal to "m"
+
+  Scenario: Update an item with a custom UUID
+    When I send the following GraphQL request:
+    """
+    mutation {
+      updateWritableId(input: {id: "/writable_ids/c6b722fe-0331-48c4-a214-f81f9f1ca082", _id: "f8a708b2-310f-416c-9aef-b1b5719dfa47", name: "Foo", clientMutationId: "m"}) {
+        id
+        _id
+        name
+        clientMutationId
+      }
+    }
+    """
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/json"
+    And the JSON node "data.updateWritableId.id" should be equal to "/writable_ids/f8a708b2-310f-416c-9aef-b1b5719dfa47"
+    And the JSON node "data.updateWritableId._id" should be equal to "f8a708b2-310f-416c-9aef-b1b5719dfa47"
+    And the JSON node "data.updateWritableId.name" should be equal to "Foo"
+    And the JSON node "data.updateWritableId.clientMutationId" should be equal to "m"
+
+  @dropSchema
   Scenario: Trigger a validation error
     When I send the following GraphQL request:
     """
diff --git a/src/Api/CachedIdentifiersExtractor.php b/src/Api/CachedIdentifiersExtractor.php
@@ -33,14 +33,20 @@ final class CachedIdentifiersExtractor implements IdentifiersExtractorInterface
     private $cacheItemPool;
     private $propertyAccessor;
     private $decorated;
+    private $resourceClassResolver;
     private $localCache = [];
     private $localResourceCache = [];
 
-    public function __construct(CacheItemPoolInterface $cacheItemPool, IdentifiersExtractorInterface $decorated, PropertyAccessorInterface $propertyAccessor = null)
+    public function __construct(CacheItemPoolInterface $cacheItemPool, IdentifiersExtractorInterface $decorated, PropertyAccessorInterface $propertyAccessor = null, ResourceClassResolverInterface $resourceClassResolver = null)
     {
         $this->cacheItemPool = $cacheItemPool;
         $this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor();
         $this->decorated = $decorated;
+        $this->resourceClassResolver = $resourceClassResolver;
+
+        if (null === $this->resourceClassResolver) {
+            @trigger_error(sprintf('Not injecting %s in the CachedIdentifiersExtractor might introduce cache issues with object identifiers.', ResourceClassResolverInterface::class), E_USER_DEPRECATED);
+        }
     }
 
     /**
@@ -77,6 +83,11 @@ public function getIdentifiersFromItem($item): array
             }
 
             $relatedResourceClass = $this->getObjectClass($identifiers[$propertyName]);
+
+            if (null !== $this->resourceClassResolver && !$this->resourceClassResolver->isResourceClass($relatedResourceClass)) {
+                continue;
+            }
+
             if (!$relatedIdentifiers = $this->localCache[$relatedResourceClass] ?? false) {
                 $relatedCacheKey = self::CACHE_KEY_PREFIX.md5($relatedResourceClass);
                 try {
diff --git a/src/Api/IdentifiersExtractor.php b/src/Api/IdentifiersExtractor.php
@@ -32,12 +32,18 @@ final class IdentifiersExtractor implements IdentifiersExtractorInterface
     private $propertyNameCollectionFactory;
     private $propertyMetadataFactory;
     private $propertyAccessor;
+    private $resourceClassResolver;
 
-    public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, PropertyAccessorInterface $propertyAccessor = null)
+    public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, PropertyAccessorInterface $propertyAccessor = null, ResourceClassResolverInterface $resourceClassResolver = null)
     {
         $this->propertyNameCollectionFactory = $propertyNameCollectionFactory;
         $this->propertyMetadataFactory = $propertyMetadataFactory;
         $this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor();
+        $this->resourceClassResolver = $resourceClassResolver;
+
+        if (null === $this->resourceClassResolver) {
+            @trigger_error(sprintf('Not injecting %s in the CachedIdentifiersExtractor might introduce cache issues with object identifiers.', ResourceClassResolverInterface::class), E_USER_DEPRECATED);
+        }
     }
 
     /**
@@ -70,16 +76,17 @@ public function getIdentifiersFromItem($item): array
             }
 
             $identifier = $identifiers[$propertyName] = $this->propertyAccessor->getValue($item, $propertyName);
+
             if (!\is_object($identifier)) {
                 continue;
             }
 
-            if (method_exists($identifier, '__toString')) {
-                $identifiers[$propertyName] = (string) $identifier;
+            $relatedResourceClass = $this->getObjectClass($identifier);
+
+            if (null !== $this->resourceClassResolver && !$this->resourceClassResolver->isResourceClass($relatedResourceClass)) {
                 continue;
             }
 
-            $relatedResourceClass = $this->getObjectClass($identifier);
             $relatedItem = $identifier;
             unset($identifiers[$propertyName]);
             foreach ($this->propertyNameCollectionFactory->create($relatedResourceClass) as $relatedPropertyName) {
diff --git a/src/Api/ResourceClassResolver.php b/src/Api/ResourceClassResolver.php
@@ -28,6 +28,7 @@ final class ResourceClassResolver implements ResourceClassResolverInterface
     use ClassInfoTrait;
 
     private $resourceNameCollectionFactory;
+    private $localIsResourceClassCache = [];
 
     public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory)
     {
@@ -68,12 +69,16 @@ public function getResourceClass($value, string $resourceClass = null, bool $str
      */
     public function isResourceClass(string $type): bool
     {
+        if (isset($this->localIsResourceClassCache[$type])) {
+            return $this->localIsResourceClassCache[$type];
+        }
+
         foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) {
             if ($type === $resourceClass) {
-                return true;
+                return $this->localIsResourceClassCache[$type] = true;
             }
         }
 
-        return false;
+        return $this->localIsResourceClassCache[$type] = false;
     }
 }
diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php
@@ -33,6 +33,7 @@
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Extension\PrependExtensionInterface;
 use Symfony\Component\DependencyInjection\Loader\XmlFileLoader;
+use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
 use Symfony\Component\Finder\Finder;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 use Symfony\Component\Validator\Validator\ValidatorInterface;
@@ -115,6 +116,9 @@ public function load(array $configs, ContainerBuilder $container)
 
         $bundles = $container->getParameter('kernel.bundles');
         if (isset($bundles['SecurityBundle'])) {
+            if (class_exists(ExpressionLanguage::class)) {
+                $loader->load('security_expression_language.xml');
+            }
             $loader->load('security.xml');
         }
 
diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml
@@ -217,12 +217,14 @@
             <argument type="service" id="api_platform.metadata.property.name_collection_factory" />
             <argument type="service" id="api_platform.metadata.property.metadata_factory" />
             <argument type="service" id="api_platform.property_accessor" />
+            <argument type="service" id="api_platform.resource_class_resolver" />
         </service>
 
         <service id="api_platform.identifiers_extractor.cached" class="ApiPlatform\Core\Api\CachedIdentifiersExtractor" decorates="api_platform.identifiers_extractor" public="false">
             <argument type="service" id="api_platform.cache.identifiers_extractor" />
             <argument type="service" id="api_platform.identifiers_extractor.cached.inner" />
             <argument type="service" id="api_platform.property_accessor" />
+            <argument type="service" id="api_platform.resource_class_resolver" />
         </service>
 
         <!-- Subresources -->
diff --git a/src/Bridge/Symfony/Bundle/Resources/config/security.xml b/src/Bridge/Symfony/Bundle/Resources/config/security.xml
@@ -5,8 +5,6 @@
            xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
 
     <services>
-        <service id="api_platform.security.expression_language" class="ApiPlatform\Core\Security\ExpressionLanguage" public="false" />
-
         <service id="api_platform.security.resource_access_checker" class="ApiPlatform\Core\Security\ResourceAccessChecker" public="false">
             <argument type="service" id="api_platform.security.expression_language" on-invalid="null" />
             <argument type="service" id="security.authentication.trust_resolver" on-invalid="null" />
diff --git a/src/Bridge/Symfony/Bundle/Resources/config/security_expression_language.xml b/src/Bridge/Symfony/Bundle/Resources/config/security_expression_language.xml
@@ -0,0 +1,9 @@
+<?xml version="1.0" ?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+           xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+           xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd">
+    <services>
+        <service id="api_platform.security.expression_language" class="ApiPlatform\Core\Security\ExpressionLanguage" public="false" />
+    </services>
+</container>
diff --git a/src/DataProvider/ChainSubresourceDataProvider.php b/src/DataProvider/ChainSubresourceDataProvider.php
@@ -37,14 +37,19 @@ public function __construct(array $dataProviders)
      */
     public function getSubresource(string $resourceClass, array $identifiers, array $context, string $operationName = null)
     {
-        foreach ($this->dataProviders as $dataProviders) {
+        foreach ($this->dataProviders as $dataProvider) {
             try {
-                return $dataProviders->getSubresource($resourceClass, $identifiers, $context, $operationName);
+                if ($dataProvider instanceof RestrictedDataProviderInterface && !$dataProvider->supports($resourceClass, $operationName, $context)) {
+                    continue;
+                }
+
+                return $dataProvider->getSubresource($resourceClass, $identifiers, $context, $operationName);
             } catch (ResourceClassNotSupportedException $e) {
+                @trigger_error(sprintf('Throwing a "%s" in a data provider is deprecated in favor of implementing "%s"', ResourceClassNotSupportedException::class, RestrictedDataProviderInterface::class), E_USER_DEPRECATED);
                 continue;
             }
         }
 
-        return null;
+        return ($context['collection'] ?? false) ? [] : null;
     }
 }
diff --git a/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php
@@ -89,7 +89,6 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
             switch ($operationName) {
                 case 'create':
                 case 'update':
-                    unset($args['input']['id']);
                     $context = null === $item ? ['resource_class' => $resourceClass] : ['resource_class' => $resourceClass, 'object_to_populate' => $item];
                     $item = $this->normalizer->denormalize($args['input'], $resourceClass, ItemNormalizer::FORMAT, $context);
                     $this->validate($item, $info, $resourceMetadata, $operationName);
diff --git a/src/GraphQl/Serializer/ItemNormalizer.php b/src/GraphQl/Serializer/ItemNormalizer.php
@@ -23,18 +23,26 @@
 
 use ApiPlatform\Core\Metadata\Property\PropertyMetadata;
 use ApiPlatform\Core\Serializer\AbstractItemNormalizer;
-use ApiPlatform\Core\Serializer\ItemNormalizer as GenericItemNormalizer;
+use ApiPlatform\Core\Serializer\ItemNormalizer as BaseItemNormalizer;
 
 /**
  * GraphQL normalizer.
  *
  * @author Kévin Dunglas <dunglas@gmail.com>
  */
-final class ItemNormalizer extends GenericItemNormalizer
+final class ItemNormalizer extends BaseItemNormalizer
 {
     const FORMAT = 'graphql';
     const ITEM_KEY = '#item';
 
+    /**
+     * {@inheritdoc}
+     */
+    public function supportsNormalization($data, $format = null)
+    {
+        return self::FORMAT === $format && parent::supportsNormalization($data, $format);
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -58,16 +66,35 @@ protected function normalizeCollectionOfRelations(PropertyMetadata $propertyMeta
     /**
      * {@inheritdoc}
      */
-    public function supportsNormalization($data, $format = null)
+    public function supportsDenormalization($data, $type, $format = null)
     {
-        return self::FORMAT === $format && parent::supportsNormalization($data, $format);
+        return self::FORMAT === $format && parent::supportsDenormalization($data, $type, $format);
     }
 
     /**
      * {@inheritdoc}
      */
-    public function supportsDenormalization($data, $type, $format = null)
+    protected function getAllowedAttributes($classOrObject, array $context, $attributesAsString = false)
     {
-        return self::FORMAT === $format && parent::supportsDenormalization($data, $type, $format);
+        $allowedAttributes = parent::getAllowedAttributes($classOrObject, $context, $attributesAsString);
+
+        if (($context['api_denormalize'] ?? false) && false !== ($indexId = array_search('id', $allowedAttributes, true))) {
+            $allowedAttributes[] = '_id';
+            array_splice($allowedAttributes, $indexId, 1);
+        }
+
+        return $allowedAttributes;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    protected function setAttributeValue($object, $attribute, $value, $format = null, array $context = [])
+    {
+        if ('_id' === $attribute) {
+            $attribute = 'id';
+        }
+
+        parent::setAttributeValue($object, $attribute, $value, $format, $context);
     }
 }
diff --git a/tests/Api/CachedIdentifiersExtractorTest.php b/tests/Api/CachedIdentifiersExtractorTest.php
diff --git a/tests/Api/IdentifiersExtractorTest.php b/tests/Api/IdentifiersExtractorTest.php
diff --git a/tests/Bridge/Symfony/Routing/IriConverterTest.php b/tests/Bridge/Symfony/Routing/IriConverterTest.php
diff --git a/tests/DataProvider/ChainSubresourcedataProviderTest.php b/tests/DataProvider/ChainSubresourcedataProviderTest.php
diff --git a/tests/Fixtures/TestBundle/Entity/WritableId.php b/tests/Fixtures/TestBundle/Entity/WritableId.php

Original file line number	Diff line number	Diff line change
`@@ -28,6 +28,7 @@ final class ResourceClassResolver implements ResourceClassResolverInterface`
`28`	`28`	`use ClassInfoTrait;`
`29`	`29`
`30`	`30`	`private $resourceNameCollectionFactory;`
	`31`	`+ private $localIsResourceClassCache = [];`
`31`	`32`
`32`	`33`	`public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory)`
`33`	`34`	`{`
`@@ -68,12 +69,16 @@ public function getResourceClass($value, string $resourceClass = null, bool $str`
`68`	`69`	`*/`
`69`	`70`	`public function isResourceClass(string $type): bool`
`70`	`71`	`{`
	`72`	`+ if (isset($this->localIsResourceClassCache[$type])) {`
	`73`	`+ return $this->localIsResourceClassCache[$type];`
	`74`	`+ }`
	`75`	`+`
`71`	`76`	`foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) {`
`72`	`77`	`if ($type === $resourceClass) {`
`73`		`- return true;`
	`78`	`+ return $this->localIsResourceClassCache[$type] = true;`
`74`	`79`	`}`
`75`	`80`	`}`
`76`	`81`
`77`		`- return false;`
	`82`	`+ return $this->localIsResourceClassCache[$type] = false;`
`78`	`83`	`}`
`79`	`84`	`}`
Original file line number	Diff line number	Diff line change
`@@ -37,14 +37,19 @@ public function __construct(array $dataProviders)`
`37`	`37`	`*/`
`38`	`38`	`public function getSubresource(string $resourceClass, array $identifiers, array $context, string $operationName = null)`
`39`	`39`	`{`
`40`		`- foreach ($this->dataProviders as $dataProviders) {`
	`40`	`+ foreach ($this->dataProviders as $dataProvider) {`
`41`	`41`	`try {`
`42`		`- return $dataProviders->getSubresource($resourceClass, $identifiers, $context, $operationName);`
	`42`	`+ if ($dataProvider instanceof RestrictedDataProviderInterface && !$dataProvider->supports($resourceClass, $operationName, $context)) {`
	`43`	`+ continue;`
	`44`	`+ }`
	`45`	`+`
	`46`	`+ return $dataProvider->getSubresource($resourceClass, $identifiers, $context, $operationName);`
`43`	`47`	`} catch (ResourceClassNotSupportedException $e) {`
	`48`	`+ @trigger_error(sprintf('Throwing a "%s" in a data provider is deprecated in favor of implementing "%s"', ResourceClassNotSupportedException::class, RestrictedDataProviderInterface::class), E_USER_DEPRECATED);`
`44`	`49`	`continue;`
`45`	`50`	`}`
`46`	`51`	`}`
`47`	`52`
`48`		`- return null;`
	`53`	`+ return ($context['collection'] ?? false) ? [] : null;`
`49`	`54`	`}`
`50`	`55`	`}`