Merge pull request #2697 from GregoireHebert/fix-cache-purge

fix (cache): prevent avoidable purge

Author: soyuka

src/Bridge/Doctrine/EventListener/PurgeHttpCacheListener.php

@@ -99,6 +99,10 @@ public function onFlush(OnFlushEventArgs $eventArgs): void
      */
     public function postFlush(): void
     {
+        if (empty($this->tags)) {
+            return;
+        }
+
         $this->purger->purge($this->tags);
         $this->tags = [];
     }
@@ -107,16 +111,15 @@ private function gatherResourceAndItemTags($entity, bool $purgeItem): void
     {
         try {
             $resourceClass = $this->resourceClassResolver->getResourceClass($entity);
+            $iri = $this->iriConverter->getIriFromResourceClass($resourceClass);
+            $this->tags[$iri] = $iri;
+            if ($purgeItem) {
+                $iri = $this->iriConverter->getIriFromItem($entity);
+                $this->tags[$iri] = $iri;
+            }
         } catch (InvalidArgumentException $e) {
             return;
         }
-
-        $iri = $this->iriConverter->getIriFromResourceClass($resourceClass);
-        $this->tags[$iri] = $iri;
-        if ($purgeItem) {
-            $iri = $this->iriConverter->getIriFromItem($entity);
-            $this->tags[$iri] = $iri;
-        }
     }
 
     private function gatherRelationTags(EntityManagerInterface $em, $entity): void

tests/Bridge/Doctrine/EventListener/PurgeHttpCacheListenerTest.php

@@ -16,8 +16,10 @@
 use ApiPlatform\Core\Api\IriConverterInterface;
 use ApiPlatform\Core\Api\ResourceClassResolverInterface;
 use ApiPlatform\Core\Bridge\Doctrine\EventListener\PurgeHttpCacheListener;
+use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\HttpCache\PurgerInterface;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyNoGetOperation;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Event\OnFlushEventArgs;
@@ -47,27 +49,34 @@ public function testOnFlush()
         $toDelete2 = new Dummy();
         $toDelete2->setId(4);
 
+        $toDeleteNoPurge = new DummyNoGetOperation();
+        $toDeleteNoPurge->setId(5);
+
         $purgerProphecy = $this->prophesize(PurgerInterface::class);
         $purgerProphecy->purge(['/dummies' => '/dummies', '/dummies/1' => '/dummies/1', '/dummies/2' => '/dummies/2', '/dummies/3' => '/dummies/3', '/dummies/4' => '/dummies/4'])->shouldBeCalled();
 
         $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
         $iriConverterProphecy->getIriFromResourceClass(Dummy::class)->willReturn('/dummies')->shouldBeCalled();
+        $iriConverterProphecy->getIriFromResourceClass(DummyNoGetOperation::class)->willThrow(new InvalidArgumentException())->shouldBeCalled();
         $iriConverterProphecy->getIriFromItem($toUpdate1)->willReturn('/dummies/1')->shouldBeCalled();
         $iriConverterProphecy->getIriFromItem($toUpdate2)->willReturn('/dummies/2')->shouldBeCalled();
         $iriConverterProphecy->getIriFromItem($toDelete1)->willReturn('/dummies/3')->shouldBeCalled();
         $iriConverterProphecy->getIriFromItem($toDelete2)->willReturn('/dummies/4')->shouldBeCalled();
+        $iriConverterProphecy->getIriFromItem($toDeleteNoPurge)->shouldNotBeCalled();
 
         $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
         $resourceClassResolverProphecy->getResourceClass(Argument::type(Dummy::class))->willReturn(Dummy::class)->shouldBeCalled();
+        $resourceClassResolverProphecy->getResourceClass(Argument::type(DummyNoGetOperation::class))->willReturn(DummyNoGetOperation::class)->shouldBeCalled();
 
         $uowProphecy = $this->prophesize(UnitOfWork::class);
         $uowProphecy->getScheduledEntityInsertions()->willReturn([$toInsert1, $toInsert2])->shouldBeCalled();
         $uowProphecy->getScheduledEntityUpdates()->willReturn([$toUpdate1, $toUpdate2])->shouldBeCalled();
-        $uowProphecy->getScheduledEntityDeletions()->willReturn([$toDelete1, $toDelete2])->shouldBeCalled();
+        $uowProphecy->getScheduledEntityDeletions()->willReturn([$toDelete1, $toDelete2, $toDeleteNoPurge])->shouldBeCalled();
 
         $emProphecy = $this->prophesize(EntityManagerInterface::class);
         $emProphecy->getUnitOfWork()->willReturn($uowProphecy->reveal())->shouldBeCalled();
         $emProphecy->getClassMetadata(Dummy::class)->willReturn(new ClassMetadata(Dummy::class))->shouldBeCalled();
+        $emProphecy->getClassMetadata(DummyNoGetOperation::class)->willReturn(new ClassMetadata(DummyNoGetOperation::class))->shouldBeCalled();
         $eventArgs = new OnFlushEventArgs($emProphecy->reveal());
 
         $listener = new PurgeHttpCacheListener($purgerProphecy->reveal(), $iriConverterProphecy->reveal(), $resourceClassResolverProphecy->reveal());
@@ -111,4 +120,32 @@ public function testPreUpdate()
         $listener->preUpdate($eventArgs);
         $listener->postFlush();
     }
+
+    public function testNothingToPurge()
+    {
+        $dummyNoGetOperation = new DummyNoGetOperation();
+        $dummyNoGetOperation->setId(1);
+
+        $purgerProphecy = $this->prophesize(PurgerInterface::class);
+        $purgerProphecy->purge([])->shouldNotBeCalled();
+
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+        $iriConverterProphecy->getIriFromResourceClass(DummyNoGetOperation::class)->willThrow(new InvalidArgumentException())->shouldBeCalled();
+        $iriConverterProphecy->getIriFromItem($dummyNoGetOperation)->shouldNotBeCalled();
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->getResourceClass(Argument::type(DummyNoGetOperation::class))->willReturn(DummyNoGetOperation::class)->shouldBeCalled();
+
+        $emProphecy = $this->prophesize(EntityManagerInterface::class);
+
+        $classMetadata = new ClassMetadata(DummyNoGetOperation::class);
+        $emProphecy->getClassMetadata(DummyNoGetOperation::class)->willReturn($classMetadata)->shouldBeCalled();
+
+        $changeSet = ['lorem' => 'ipsum'];
+        $eventArgs = new PreUpdateEventArgs($dummyNoGetOperation, $emProphecy->reveal(), $changeSet);
+
+        $listener = new PurgeHttpCacheListener($purgerProphecy->reveal(), $iriConverterProphecy->reveal(), $resourceClassResolverProphecy->reveal());
+        $listener->preUpdate($eventArgs);
+        $listener->postFlush();
+    }
 }

tests/Fixtures/TestBundle/Entity/DummyNoGetOperation.php

@@ -0,0 +1,53 @@
+<?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\Entity;
+
+use ApiPlatform\Core\Annotation\ApiResource;
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * DummyNoGetOperation.
+ *
+ * @author Grégoire Hébert [email protected]
+ *
+ * @ORM\Entity
+ *
+ * @ApiResource(
+ *     collectionOperations={"post"},
+ *     itemOperations={"put"}
+ * )
+ */
+class DummyNoGetOperation
+{
+    /**
+     * @var int The id
+     *
+     * @ORM\Column(type="integer")
+     * @ORM\Id
+     * @ORM\GeneratedValue(strategy="AUTO")
+     */
+    private $id;
+
+    /**
+     * @var string
+     *
+     * @ORM\Column
+     */
+    public $lorem;
+
+    public function setId($id)
+    {
+        $this->id = $id;
+    }
+}