IBX-5502: Added additional tag cleaning to limit down number of orphaned tag entries

eZ/Publish/Core/Persistence/Cache/ContentHandler.php

@@ -328,12 +328,33 @@ public function updateContent($contentId, $versionNo, UpdateStruct $struct)
     {
         $this->logger->logCall(__METHOD__, ['content' => $contentId, 'version' => $versionNo, 'struct' => $struct]);
         $content = $this->persistenceHandler->contentHandler()->updateContent($contentId, $versionNo, $struct);
-        $this->cache->invalidateTags([
-            $this->cacheIdentifierGenerator->generateTag(
+        $locations = $this->persistenceHandler->locationHandler()->loadLocationsByContent($contentId);
+
+        $locationTags = array_map(function (Content\Location $location): string {
+            return $this->cacheIdentifierGenerator->generateTag(self::LOCATION_IDENTIFIER, [$location->id]);
+        }, $locations);
+        $locationPathTags = array_map(function (Content\Location $location): string {
+            return $this->cacheIdentifierGenerator->generateTag(self::LOCATION_PATH_IDENTIFIER, [$location->id]);
+        }, $locations);
+
+        $versionTags = [];
+        $versionTags[] = $this->cacheIdentifierGenerator->generateTag(
+            self::CONTENT_VERSION_IDENTIFIER,
+            [$contentId, $versionNo]
+        );
+        if ($versionNo > 1) {
+            $versionTags[] = $this->cacheIdentifierGenerator->generateTag(
                 self::CONTENT_VERSION_IDENTIFIER,
-                [$contentId, $versionNo]
-            ),
-        ]);
+                [$contentId, $versionNo - 1]
+            );
+        }
+
+        $tags = array_merge(
+            $locationTags,
+            $locationPathTags,
+            $versionTags
+        );
+        $this->cache->invalidateTags($tags);
 
         return $content;
     }
@@ -350,6 +371,7 @@ public function deleteContent($contentId)
             $contentId,
             APIRelation::FIELD | APIRelation::ASSET
         );
+        $contentLocations = $this->persistenceHandler->locationHandler()->loadLocationsByContent($contentId);
 
         $return = $this->persistenceHandler->contentHandler()->deleteContent($contentId);
 
@@ -365,6 +387,9 @@ function ($relation) {
             $tags = [];
         }
         $tags[] = $this->cacheIdentifierGenerator->generateTag(self::CONTENT_IDENTIFIER, [$contentId]);
+        foreach ($contentLocations as $location) {
+            $tags[] = $this->cacheIdentifierGenerator->generateTag(self::LOCATION_IDENTIFIER, [$location->id]);
+        }
         $this->cache->invalidateTags($tags);
 
         return $return;

eZ/Publish/Core/Persistence/Cache/Tests/ContentHandlerTest.php

@@ -12,11 +12,13 @@
 use eZ\Publish\SPI\Persistence\Content\ContentInfo;
 use eZ\Publish\SPI\Persistence\Content\CreateStruct;
 use eZ\Publish\SPI\Persistence\Content\Handler as SPIContentHandler;
+use eZ\Publish\SPI\Persistence\Content\Location\Handler as SPILocationHandler;
 use eZ\Publish\SPI\Persistence\Content\MetadataUpdateStruct;
 use eZ\Publish\SPI\Persistence\Content\Relation as SPIRelation;
 use eZ\Publish\SPI\Persistence\Content\Relation\CreateStruct as RelationCreateStruct;
 use eZ\Publish\SPI\Persistence\Content\UpdateStruct;
 use eZ\Publish\SPI\Persistence\Content\VersionInfo;
+use PHPUnit\Framework\MockObject\MockObject;
 
 /**
  * Test case for Persistence\Cache\ContentHandler.
@@ -47,7 +49,7 @@ public function providerForUnCachedMethods(): array
             ['setStatus', [2, 0, 1], [['content_version', [2, 1], false]], null, ['c-2-v-1']],
             ['setStatus', [2, 1, 1], [['content', [2], false]], null, ['c-2']],
             ['updateMetadata', [2, new MetadataUpdateStruct()], [['content', [2], false]], null, ['c-2']],
-            ['updateContent', [2, 1, new UpdateStruct()], [['content_version', [2, 1], false]], null, ['c-2-v-1']],
+            //['updateContent', [2, 1, new UpdateStruct()], [['content_version', [2, 1], false]], null, ['c-2-v-1']],
             //['deleteContent', [2]], own tests for relations complexity
             ['deleteVersion', [2, 1], [['content_version', [2, 1], false]], null, ['c-2-v-1']],
             ['addRelation', [new RelationCreateStruct()]],
@@ -303,30 +305,68 @@ public function providerForCachedLoadMethodsMiss(): array
         ];
     }
 
+    /**
+     * @covers \eZ\Publish\Core\Persistence\Cache\ContentHandler::updateContent
+     */
+    public function testUpdateContent()
+    {
+        $this->loggerMock->expects($this->once())->method('logCall');
+
+        $innerContentHandlerMock = $this->createMock(SPIContentHandler::class);
+        $innerLocationHandlerMock = $this->createMock(SPILocationHandler::class);
+
+        $this->prepareHandlerMocks(
+            $innerContentHandlerMock,
+            $innerLocationHandlerMock,
+            1,
+            1,
+            0
+        );
+
+        $innerContentHandlerMock
+            ->expects($this->once())
+            ->method('updateContent')
+            ->with(2, 1, new UpdateStruct())
+            ->willReturn(new Content());
+
+        $this->cacheIdentifierGeneratorMock
+            ->expects($this->exactly(5))
+            ->method('generateTag')
+            ->withConsecutive(
+                ['location', [3], false],
+                ['location', [4], false],
+                ['location_path', [3], false],
+                ['location_path', [4], false],
+                ['content_version', [2, 1], false],
+            )
+            ->willReturnOnConsecutiveCalls('l-3', 'l-4', 'lp-3', 'lp-4', 'c-2-v-1');
+
+        $this->cacheMock
+            ->expects($this->once())
+            ->method('invalidateTags')
+            ->with(['l-3', 'l-4', 'lp-3', 'lp-4', 'c-2-v-1']);
+
+        $handler = $this->persistenceCacheHandler->contentHandler();
+        $handler->updateContent(2, 1, new UpdateStruct());
+    }
+
     /**
      * @covers \eZ\Publish\Core\Persistence\Cache\ContentHandler::deleteContent
      */
     public function testDeleteContent()
     {
         $this->loggerMock->expects($this->once())->method('logCall');
 
-        $innerHandlerMock = $this->createMock(SPIContentHandler::class);
-        $this->persistenceHandlerMock
-            ->expects($this->exactly(2))
-            ->method('contentHandler')
-            ->willReturn($innerHandlerMock);
+        $innerContentHandlerMock = $this->createMock(SPIContentHandler::class);
+        $innerLocationHandlerMock = $this->createMock(SPILocationHandler::class);
 
-        $innerHandlerMock
-            ->expects($this->once())
-            ->method('loadReverseRelations')
-            ->with(2, APIRelation::FIELD | APIRelation::ASSET)
-            ->willReturn(
-                [
-                    new SPIRelation(['sourceContentId' => 42]),
-                ]
-            );
+        $this->prepareHandlerMocks(
+            $innerContentHandlerMock,
+            $innerLocationHandlerMock,
+            2
+        );
 
-        $innerHandlerMock
+        $innerContentHandlerMock
             ->expects($this->once())
             ->method('deleteContent')
             ->with(2)
@@ -337,20 +377,62 @@ public function testDeleteContent()
             ->method('deleteItem');
 
         $this->cacheIdentifierGeneratorMock
-            ->expects($this->exactly(2))
+            ->expects($this->exactly(4))
             ->method('generateTag')
             ->withConsecutive(
                 ['content', [42], false],
-                ['content', [2], false]
+                ['content', [2], false],
+                ['location', [3], false],
+                ['location', [4], false]
             )
-            ->willReturnOnConsecutiveCalls('c-42', 'c-2');
+            ->willReturnOnConsecutiveCalls('c-42', 'c-2', 'l-3', 'l-4');
 
         $this->cacheMock
             ->expects($this->once())
             ->method('invalidateTags')
-            ->with(['c-42', 'c-2']);
+            ->with(['c-42', 'c-2', 'l-3', 'l-4']);
 
         $handler = $this->persistenceCacheHandler->contentHandler();
         $handler->deleteContent(2);
     }
+
+    private function prepareHandlerMocks(
+        MockObject $innerContentHandlerMock,
+        MockObject $innerLocationHandlerMock,
+        int $contentHandlerCount = 1,
+        int $locationHandlerCount = 1,
+        int $loadReverseRelationsCount = 1,
+        int $loadLocationsByContentCount = 1
+    ): void {
+        $this->persistenceHandlerMock
+            ->expects($this->exactly($contentHandlerCount))
+            ->method('contentHandler')
+            ->willReturn($innerContentHandlerMock);
+
+        $innerContentHandlerMock
+            ->expects($this->exactly($loadReverseRelationsCount))
+            ->method('loadReverseRelations')
+            ->with(2, APIRelation::FIELD | APIRelation::ASSET)
+            ->willReturn(
+                [
+                    new SPIRelation(['sourceContentId' => 42]),
+                ]
+            );
+
+        $this->persistenceHandlerMock
+            ->expects($this->exactly($locationHandlerCount))
+            ->method('locationHandler')
+            ->willReturn($innerLocationHandlerMock);
+
+        $innerLocationHandlerMock
+            ->expects($this->exactly($loadLocationsByContentCount))
+            ->method('loadLocationsByContent')
+            ->with(2)
+            ->willReturn(
+                [
+                    new Content\Location(['id' => 3]),
+                    new Content\Location(['id' => 4]),
+                ]
+            );
+    }
 }

eZ/Publish/Core/Persistence/Cache/Tests/TrashHandlerTest.php

@@ -13,6 +13,7 @@
 use eZ\Publish\SPI\Persistence\Content\Location\Trash\Handler as TrashHandler;
 use eZ\Publish\SPI\Persistence\Content\Location\Trashed;
 use eZ\Publish\SPI\Persistence\Content\Relation;
+use eZ\Publish\SPI\Persistence\Content\VersionInfo;
 
 /**
  * Test case for Persistence\Cache\SectionHandler.
@@ -118,10 +119,13 @@ public function testTrashSubtree()
     {
         $locationId = 6;
         $contentId = 42;
+        $versionNo = 1;
 
         $tags = [
+            'c-' . $contentId . '-v-' . $versionNo,
             'c-' . $contentId,
             'lp-' . $locationId,
+            'l-' . $locationId,
         ];
 
         $handlerMethodName = $this->getHandlerMethodName();
@@ -144,6 +148,16 @@ public function testTrashSubtree()
             ->method('locationHandler')
             ->willReturn($locationHandlerMock);
 
+        $contentHandlerMock
+            ->expects($this->once())
+            ->method('listVersions')
+            ->with($contentId)
+            ->willReturn(
+                [
+                    new VersionInfo(['versionNo' => $versionNo]),
+                ]
+            );
+
         $this->persistenceHandlerMock
             ->expects($this->once())
             ->method($handlerMethodName)
@@ -154,13 +168,14 @@ public function testTrashSubtree()
             ->method('trashSubtree')
             ->with($locationId)
             ->willReturn(null);
-
         $this->cacheIdentifierGeneratorMock
-            ->expects($this->exactly(2))
+            ->expects($this->exactly(4))
             ->method('generateTag')
             ->withConsecutive(
+                ['content_version', [$contentId, $versionNo], false],
                 ['content', [$contentId], false],
-                ['location_path', [$locationId], false]
+                ['location_path', [$locationId], false],
+                ['location', [$locationId], false]
             )
             ->willReturnOnConsecutiveCalls(...$tags);
 

eZ/Publish/Core/Persistence/Cache/TrashHandler.php

@@ -9,6 +9,7 @@
 use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
 use eZ\Publish\SPI\Persistence\Content\Location\Trash\Handler as TrashHandlerInterface;
 use eZ\Publish\SPI\Persistence\Content\Relation;
+use eZ\Publish\SPI\Persistence\Content\VersionInfo;
 
 /**
  * @see \eZ\Publish\SPI\Persistence\Content\Location\Trash\Handler
@@ -17,6 +18,8 @@ class TrashHandler extends AbstractHandler implements TrashHandlerInterface
 {
     private const EMPTY_TRASH_BULK_SIZE = 100;
     private const CONTENT_IDENTIFIER = 'content';
+    private const CONTENT_VERSION_IDENTIFIER = 'content_version';
+    private const LOCATION_IDENTIFIER = 'location';
     private const LOCATION_PATH_IDENTIFIER = 'location_path';
 
     /**
@@ -37,8 +40,9 @@ public function trashSubtree($locationId)
         $this->logger->logCall(__METHOD__, ['locationId' => $locationId]);
 
         $location = $this->persistenceHandler->locationHandler()->load($locationId);
-        $reverseRelations = $this->persistenceHandler->contentHandler()->loadRelations($location->contentId);
-
+        $contentId = $location->contentId;
+        $reverseRelations = $this->persistenceHandler->contentHandler()->loadRelations($contentId);
+        $versions = $this->persistenceHandler->contentHandler()->listVersions($contentId);
         $return = $this->persistenceHandler->trashHandler()->trashSubtree($locationId);
 
         $relationTags = [];
@@ -51,13 +55,23 @@ public function trashSubtree($locationId)
             }, $reverseRelations);
         }
 
+        $versionTags = array_map(function (VersionInfo $versionInfo) use ($contentId): string {
+            return  $this->cacheIdentifierGenerator->generateTag(
+                self::CONTENT_VERSION_IDENTIFIER,
+                [$contentId, $versionInfo->versionNo]
+            );
+        }, $versions);
+
         $tags = array_merge(
+            $versionTags,
+            $relationTags,
             [
-                $this->cacheIdentifierGenerator->generateTag(self::CONTENT_IDENTIFIER, [$location->contentId]),
+                $this->cacheIdentifierGenerator->generateTag(self::CONTENT_IDENTIFIER, [$contentId]),
                 $this->cacheIdentifierGenerator->generateTag(self::LOCATION_PATH_IDENTIFIER, [$locationId]),
+                $this->cacheIdentifierGenerator->generateTag(self::LOCATION_IDENTIFIER, [$locationId]),
             ],
-            $relationTags
         );
+
         $this->cache->invalidateTags(array_values(array_unique($tags)));
 
         return $return;