Merge branch '2.4'

Author: dunglas

features/authorization/deny.feature

@@ -16,6 +16,12 @@ Feature: Authorization checking
     Then the response status code should be 200
     And the response should be in JSON
 
+  Scenario: Data provider that's return generator has null previous object
+    When I add "Accept" header equal to "application/ld+json"
+    And I add "Authorization" header equal to "Basic ZHVuZ2xhczprZXZpbg=="
+    And I send a "GET" request to "/custom_data_provider_generator"
+    Then the response status code should be 200
+
   Scenario: A standard user cannot create a secured resource
     When I add "Accept" header equal to "application/ld+json"
     And I add "Content-Type" header equal to "application/ld+json"

src/EventListener/ReadListener.php

@@ -23,6 +23,7 @@
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Resource\ToggleableOperationAttributeTrait;
 use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface;
+use ApiPlatform\Core\Util\CloneTrait;
 use ApiPlatform\Core\Util\RequestAttributesExtractor;
 use ApiPlatform\Core\Util\RequestParser;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
@@ -35,6 +36,7 @@
  */
 final class ReadListener
 {
+    use CloneTrait;
     use OperationDataProviderTrait;
     use ToggleableOperationAttributeTrait;
 
@@ -115,6 +117,6 @@ public function onKernelRequest(GetResponseEvent $event): void
         }
 
         $request->attributes->set('data', $data);
-        $request->attributes->set('previous_data', \is_object($data) ? clone $data : $data);
+        $request->attributes->set('previous_data', $this->clone($data));
     }
 }

src/GraphQl/Resolver/Factory/CollectionResolverFactory.php

@@ -18,6 +18,7 @@
 use ApiPlatform\Core\GraphQl\Resolver\Stage\ReadStageInterface;
 use ApiPlatform\Core\GraphQl\Resolver\Stage\SerializeStageInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
+use ApiPlatform\Core\Util\CloneTrait;
 use GraphQL\Type\Definition\ResolveInfo;
 use Psr\Container\ContainerInterface;
 use Symfony\Component\HttpFoundation\RequestStack;
@@ -32,6 +33,8 @@
  */
 final class CollectionResolverFactory implements ResolverFactoryInterface
 {
+    use CloneTrait;
+
     private $readStage;
     private $denyAccessStage;
     private $serializeStage;
@@ -83,7 +86,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
             ($this->denyAccessStage)($resourceClass, $operationName, $resolverContext + [
                 'extra_variables' => [
                     'object' => $collection,
-                    'previous_object' => \is_object($collection) ? clone $collection : $collection,
+                    'previous_object' => $this->clone($collection),
                 ],
             ]);
 

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

@@ -22,6 +22,7 @@
 use ApiPlatform\Core\GraphQl\Resolver\Stage\WriteStageInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Util\ClassInfoTrait;
+use ApiPlatform\Core\Util\CloneTrait;
 use GraphQL\Error\Error;
 use GraphQL\Type\Definition\ResolveInfo;
 use Psr\Container\ContainerInterface;
@@ -36,6 +37,7 @@
 final class ItemMutationResolverFactory implements ResolverFactoryInterface
 {
     use ClassInfoTrait;
+    use CloneTrait;
 
     private $readStage;
     private $denyAccessStage;
@@ -71,8 +73,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul
             if (null !== $item && !\is_object($item)) {
                 throw new \LogicException('Item from read stage should be a nullable object.');
             }
-
-            $previousItem = \is_object($item) ? clone $item : $item;
+            $previousItem = $this->clone($item);
 
             if ('delete' === $operationName) {
                 ($this->denyAccessStage)($resourceClass, $operationName, $resolverContext + [

src/Util/CloneTrait.php

@@ -0,0 +1,37 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\Core\Util;
+
+/**
+ * Clones given data if cloneable.
+ *
+ * @internal
+ *
+ * @author Quentin Barloy <[email protected]>
+ */
+trait CloneTrait
+{
+    public function clone($data)
+    {
+        if (!\is_object($data)) {
+            return $data;
+        }
+
+        try {
+            return (new \ReflectionClass($data))->isCloneable() ? clone $data : null;
+        } catch (\ReflectionException $reflectionException) {
+            return null;
+        }
+    }
+}

tests/Fixtures/TestBundle/DataProvider/GeneratorDataProvider.php

@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\Core\Tests\Fixtures\TestBundle\DataProvider;
+
+use ApiPlatform\Core\DataProvider\CollectionDataProviderInterface;
+use ApiPlatform\Core\DataProvider\RestrictedDataProviderInterface;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\SecuredDummy;
+
+class GeneratorDataProvider implements CollectionDataProviderInterface, RestrictedDataProviderInterface
+{
+    public function supports(string $resourceClass, string $operationName = null, array $context = []): bool
+    {
+        return SecuredDummy::class === $resourceClass && 'get_from_data_provider_generator' === $operationName;
+    }
+
+    public function getCollection(string $resourceClass, string $operationName = null)
+    {
+        yield from [new class() {
+        }, new class() {
+        }];
+    }
+}

tests/Fixtures/TestBundle/Document/SecuredDummy.php

@@ -27,6 +27,11 @@
  *     attributes={"access_control"="is_granted('ROLE_USER')"},
  *     collectionOperations={
  *         "get",
+ *         "get_from_data_provider_generator"={
+ *             "method"="GET",
+ *             "path"="custom_data_provider_generator",
+ *             "access_control"="is_granted('ROLE_USER')"
+ *         },
  *         "post"={"access_control"="is_granted('ROLE_ADMIN')"}
  *     },
  *     itemOperations={

tests/Fixtures/TestBundle/Entity/SecuredDummy.php

@@ -26,6 +26,11 @@
  *     attributes={"access_control"="is_granted('ROLE_USER')"},
  *     collectionOperations={
  *         "get",
+ *         "get_from_data_provider_generator"={
+ *             "method"="GET",
+ *             "path"="custom_data_provider_generator",
+ *             "access_control"="is_granted('ROLE_USER')"
+ *         },
  *         "post"={"access_control"="is_granted('ROLE_ADMIN')"}
  *     },
  *     itemOperations={

tests/Fixtures/app/config/config_test.yml

@@ -73,6 +73,11 @@ services:
         arguments: [ { 'name': 'ipartial', 'description': 'ipartial' } ]
         tags:      [ { name: 'api_platform.filter', id: 'related_to_dummy_friend.name' } ]
 
+    ApiPlatform\Core\Tests\Fixtures\TestBundle\DataProvider\GeneratorDataProvider:
+        public: false
+        tags:
+            -   name: 'api_platform.item_data_provider'
+
     ApiPlatform\Core\Tests\Fixtures\TestBundle\DataProvider\ProductItemDataProvider:
         public: false
         arguments:

tests/Util/CloneTraitTest.php

@@ -0,0 +1,49 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\Core\Tests\Util;
+
+use ApiPlatform\Core\Util\CloneTrait;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @author Quentin Barloy <[email protected]>
+ */
+class CloneTraitTest extends TestCase
+{
+    use CloneTrait;
+
+    public function testScalarClone(): void
+    {
+        $this->assertSame(5, $this->clone(5));
+    }
+
+    public function testObjectClone(): void
+    {
+        $data = new \stdClass();
+        $result = $this->clone($data);
+
+        $this->assertNotSame($data, $result);
+        $this->assertEquals($data, $result);
+    }
+
+    public function testGeneratorClone(): void
+    {
+        $this->assertNull($this->clone($this->generator()));
+    }
+
+    private function generator(): \Generator
+    {
+        yield 1;
+    }
+}