From 6e60c002758124f2a2e0f594093a0b50962e69ec Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Wed, 12 Feb 2025 14:32:42 +0100 Subject: [PATCH 01/10] Added additional tag cleaning to limit down number of orphaned tag entries --- src/lib/Persistence/Cache/ContentHandler.php | 36 +++++- src/lib/Persistence/Cache/TrashHandler.php | 23 +++- .../Persistence/Cache/ContentHandlerTest.php | 121 ++++++++++++++---- .../Persistence/Cache/TrashHandlerTest.php | 19 ++- 4 files changed, 165 insertions(+), 34 deletions(-) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index 68d8027f02..d0d167c89e 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -335,12 +335,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; } @@ -357,6 +378,7 @@ public function deleteContent($contentId) $contentId, APIRelation::FIELD | APIRelation::ASSET ); + $contentLocations = $this->persistenceHandler->locationHandler()->loadLocationsByContent($contentId); $return = $this->persistenceHandler->contentHandler()->deleteContent($contentId); @@ -372,7 +394,11 @@ function ($relation) { $tags = []; } $tags[] = $this->cacheIdentifierGenerator->generateTag(self::CONTENT_IDENTIFIER, [$contentId]); + $this->cache->invalidateTags($tags); + foreach ($contentLocations as $location) { + $tags[] = $this->cacheIdentifierGenerator->generateTag(self::LOCATION_IDENTIFIER, [$location->id]); + } return $return; } diff --git a/src/lib/Persistence/Cache/TrashHandler.php b/src/lib/Persistence/Cache/TrashHandler.php index a8f5fdd39f..23a4c7b144 100644 --- a/src/lib/Persistence/Cache/TrashHandler.php +++ b/src/lib/Persistence/Cache/TrashHandler.php @@ -8,12 +8,15 @@ use Ibexa\Contracts\Core\Persistence\Content\Location\Trash\Handler as TrashHandlerInterface; use Ibexa\Contracts\Core\Persistence\Content\Relation; +use Ibexa\Contracts\Core\Persistence\Content\VersionInfo; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; 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'; /** @@ -34,8 +37,11 @@ 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; + $contentHandler = $this->persistenceHandler->contentHandler(); + $reverseRelations = $contentHandler->loadRelations($contentId); + $versions = $contentHandler->listVersions($contentId); $return = $this->persistenceHandler->trashHandler()->trashSubtree($locationId); $relationTags = []; @@ -48,12 +54,21 @@ 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]), - ], - $relationTags + $this->cacheIdentifierGenerator->generateTag(self::LOCATION_IDENTIFIER, [$locationId]), + ] ); $this->cache->invalidateTags(array_values(array_unique($tags))); diff --git a/tests/lib/Persistence/Cache/ContentHandlerTest.php b/tests/lib/Persistence/Cache/ContentHandlerTest.php index 7ce7d7bd6a..b66a489143 100644 --- a/tests/lib/Persistence/Cache/ContentHandlerTest.php +++ b/tests/lib/Persistence/Cache/ContentHandlerTest.php @@ -48,7 +48,8 @@ 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 has its own test now due to relations complexity + //['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(['destinationContentId' => 2, 'sourceContentId' => 4])], [['content', [2], false], ['content', [4], false]], null, ['c-2', 'c-4']], @@ -437,53 +438,127 @@ public function providerForCachedLoadMethodsMiss(): array ]; } - public function testDeleteContent() + public function testUpdateContent(): void { $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 + $this->prepareHandlerMocks( + $innerContentHandlerMock, + $innerLocationHandlerMock, + 1, + 1, + 0 + ); + + $innerContentHandlerMock ->expects($this->once()) - ->method('loadReverseRelations') - ->with(2, APIRelation::FIELD | APIRelation::ASSET) - ->willReturn( - [ - new SPIRelation(['sourceContentId' => 42]), - ] + ->method('updateContent') + ->with(2, 1, new UpdateStruct()) + ->willReturn(new Content()); + + $this->cacheIdentifierGeneratorMock + ->expects($this->exactly(5)) + ->method('generateTag') + ->will( + self::returnValueMap([ + ['location', [3], false, 'l-3'], + ['location', [4], false, 'l-4'], + ['location_path', [3], false, 'lp-3'], + ['location_path', [4], false, 'lp-4'], + ['content_version', [2, 1], false, 'c-2-v-1'], + ]) ); - $innerHandlerMock + $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()); + } + + public function testDeleteContent(): void + { + $this->loggerMock->expects($this->once())->method('logCall'); + + $innerHandlerMock = $this->createMock(SPIContentHandler::class); + $innerLocationHandlerMock = $this->createMock(SPILocationHandler::class); + + $this->prepareHandlerMocks( + $innerContentHandlerMock, + $innerLocationHandlerMock, + 2 + ); + + $innerContentHandlerMock ->expects($this->once()) ->method('deleteContent') ->with(2) - ->willReturn(true); - - $this->cacheMock - ->expects($this->never()) ->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]), + ] + ); + } } class_alias(ContentHandlerTest::class, 'eZ\Publish\Core\Persistence\Cache\Tests\ContentHandlerTest'); diff --git a/tests/lib/Persistence/Cache/TrashHandlerTest.php b/tests/lib/Persistence/Cache/TrashHandlerTest.php index dcfc8712bd..063a0d5f76 100644 --- a/tests/lib/Persistence/Cache/TrashHandlerTest.php +++ b/tests/lib/Persistence/Cache/TrashHandlerTest.php @@ -13,6 +13,7 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Trash\TrashItemDeleteResult; use Ibexa\Core\Persistence\Cache\ContentHandler; use Ibexa\Core\Persistence\Cache\LocationHandler; +use Ibexa\Core\Persistence\Cache\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(); @@ -149,6 +153,15 @@ public function testTrashSubtree() ->method($handlerMethodName) ->willReturn($innerHandler); + $contentHandlerMock + ->expects($this->once()) + ->method('listVersions') + ->with($contentId) + ->willReturn( + [ + new VersionInfo(['versionNo' => $versionNo]), + ] + ); $innerHandler ->expects($this->once()) ->method('trashSubtree') @@ -156,11 +169,13 @@ public function testTrashSubtree() ->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); From 06b7bce1d4e3e81d121c2c527551acf4dda4ac8a Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Wed, 12 Feb 2025 14:52:23 +0100 Subject: [PATCH 02/10] Fixed PHPStan errors --- tests/lib/Persistence/Cache/ContentHandlerTest.php | 13 +++++++++---- tests/lib/Persistence/Cache/TrashHandlerTest.php | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tests/lib/Persistence/Cache/ContentHandlerTest.php b/tests/lib/Persistence/Cache/ContentHandlerTest.php index b66a489143..86998a34d0 100644 --- a/tests/lib/Persistence/Cache/ContentHandlerTest.php +++ b/tests/lib/Persistence/Cache/ContentHandlerTest.php @@ -10,6 +10,7 @@ use Ibexa\Contracts\Core\Persistence\Content\ContentInfo; use Ibexa\Contracts\Core\Persistence\Content\CreateStruct; use Ibexa\Contracts\Core\Persistence\Content\Handler as SPIContentHandler; +use Ibexa\Contracts\Core\Persistence\Content\Location\Handler as SPILocationHandler; use Ibexa\Contracts\Core\Persistence\Content\MetadataUpdateStruct; use Ibexa\Contracts\Core\Persistence\Content\Relation; use Ibexa\Contracts\Core\Persistence\Content\Relation as SPIRelation; @@ -481,11 +482,11 @@ public function testUpdateContent(): void $handler->updateContent(2, 1, new UpdateStruct()); } - public function testDeleteContent(): void + public function testDeleteContent() { $this->loggerMock->expects($this->once())->method('logCall'); - $innerHandlerMock = $this->createMock(SPIContentHandler::class); + $innerContentHandlerMock = $this->createMock(SPIContentHandler::class); $innerLocationHandlerMock = $this->createMock(SPILocationHandler::class); $this->prepareHandlerMocks( @@ -520,9 +521,13 @@ public function testDeleteContent(): void $handler->deleteContent(2); } + /** + * @param SPIContentHandler|\PHPUnit\Framework\MockObject\MockObject $innerContentHandlerMock + * @param SPILocationHandler|\PHPUnit\Framework\MockObject\MockObject $innerLocationHandlerMock + */ private function prepareHandlerMocks( - MockObject $innerContentHandlerMock, - MockObject $innerLocationHandlerMock, + SPIContentHandler $innerContentHandlerMock, + SPILocationHandler $innerLocationHandlerMock, int $contentHandlerCount = 1, int $locationHandlerCount = 1, int $loadReverseRelationsCount = 1, diff --git a/tests/lib/Persistence/Cache/TrashHandlerTest.php b/tests/lib/Persistence/Cache/TrashHandlerTest.php index 063a0d5f76..c88a92b65f 100644 --- a/tests/lib/Persistence/Cache/TrashHandlerTest.php +++ b/tests/lib/Persistence/Cache/TrashHandlerTest.php @@ -10,10 +10,10 @@ use Ibexa\Contracts\Core\Persistence\Content\Location\Trash\Handler as TrashHandler; use Ibexa\Contracts\Core\Persistence\Content\Location\Trashed; use Ibexa\Contracts\Core\Persistence\Content\Relation; +use Ibexa\Contracts\Core\Persistence\Content\VersionInfo; use Ibexa\Contracts\Core\Repository\Values\Content\Trash\TrashItemDeleteResult; use Ibexa\Core\Persistence\Cache\ContentHandler; use Ibexa\Core\Persistence\Cache\LocationHandler; -use Ibexa\Core\Persistence\Cache\VersionInfo; /** * Test case for Persistence\Cache\SectionHandler. From e6ce8b7523e4edfefb972e480ca44a6aa05624e9 Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Wed, 12 Feb 2025 14:59:21 +0100 Subject: [PATCH 03/10] CS Fix --- tests/lib/Persistence/Cache/ContentHandlerTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/lib/Persistence/Cache/ContentHandlerTest.php b/tests/lib/Persistence/Cache/ContentHandlerTest.php index 86998a34d0..4d5fdbc781 100644 --- a/tests/lib/Persistence/Cache/ContentHandlerTest.php +++ b/tests/lib/Persistence/Cache/ContentHandlerTest.php @@ -522,8 +522,8 @@ public function testDeleteContent() } /** - * @param SPIContentHandler|\PHPUnit\Framework\MockObject\MockObject $innerContentHandlerMock - * @param SPILocationHandler|\PHPUnit\Framework\MockObject\MockObject $innerLocationHandlerMock + * @param \Ibexa\Contracts\Core\Persistence\Content\Handler|\PHPUnit\Framework\MockObject\MockObject $innerContentHandlerMock + * @param \Ibexa\Contracts\Core\Persistence\Content\Location\Handler|\PHPUnit\Framework\MockObject\MockObject $innerLocationHandlerMock */ private function prepareHandlerMocks( SPIContentHandler $innerContentHandlerMock, From 65365329fbceb95eee1cf1c3f7d79f1f4098f790 Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Thu, 13 Feb 2025 11:38:35 +0100 Subject: [PATCH 04/10] Fixing tests --- tests/lib/Persistence/Cache/ContentHandlerTest.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/lib/Persistence/Cache/ContentHandlerTest.php b/tests/lib/Persistence/Cache/ContentHandlerTest.php index 4d5fdbc781..53f13b67ee 100644 --- a/tests/lib/Persistence/Cache/ContentHandlerTest.php +++ b/tests/lib/Persistence/Cache/ContentHandlerTest.php @@ -499,6 +499,10 @@ public function testDeleteContent() ->expects($this->once()) ->method('deleteContent') ->with(2) + ->willReturn(true); + + $this->cacheMock + ->expects($this->never()) ->method('deleteItem'); $this->cacheIdentifierGeneratorMock From b501ee2637d568a8f6f91638454157d2ffa23e2a Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Thu, 13 Feb 2025 12:34:24 +0100 Subject: [PATCH 05/10] Fixed tests --- src/lib/Persistence/Cache/ContentHandler.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index d0d167c89e..7bec6935fb 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -394,11 +394,11 @@ function ($relation) { $tags = []; } $tags[] = $this->cacheIdentifierGenerator->generateTag(self::CONTENT_IDENTIFIER, [$contentId]); - - $this->cache->invalidateTags($tags); foreach ($contentLocations as $location) { $tags[] = $this->cacheIdentifierGenerator->generateTag(self::LOCATION_IDENTIFIER, [$location->id]); } + + $this->cache->invalidateTags($tags); return $return; } From d597ba70a29c9bf2cb7f9786d59e97d7f06e3e95 Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Thu, 13 Feb 2025 12:46:10 +0100 Subject: [PATCH 06/10] Fixed CS --- src/lib/Persistence/Cache/ContentHandler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index 7bec6935fb..bc735cfcde 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -397,7 +397,7 @@ function ($relation) { foreach ($contentLocations as $location) { $tags[] = $this->cacheIdentifierGenerator->generateTag(self::LOCATION_IDENTIFIER, [$location->id]); } - + $this->cache->invalidateTags($tags); return $return; From a71ea65f4af8d7515906dfbbca285befe6e42bb2 Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Mon, 31 Mar 2025 10:10:28 +0200 Subject: [PATCH 07/10] Changes after CR#1 --- src/lib/Persistence/Cache/ContentHandler.php | 6 ++- .../Persistence/Cache/ContentHandlerTest.php | 40 +++++++++---------- .../Persistence/Cache/TrashHandlerTest.php | 10 ++--- 3 files changed, 29 insertions(+), 27 deletions(-) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index bc735cfcde..d0de048ede 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -4,6 +4,7 @@ * @copyright Copyright (C) Ibexa AS. All rights reserved. * @license For full copyright and license information view LICENSE file distributed with this source code. */ + namespace Ibexa\Core\Persistence\Cache; use Ibexa\Contracts\Core\Persistence\Content; @@ -329,9 +330,10 @@ public function updateMetadata($contentId, MetadataUpdateStruct $struct) } /** - * {@inheritdoc} + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException + * @throws \Psr\Cache\InvalidArgumentException */ - public function updateContent($contentId, $versionNo, UpdateStruct $struct) + public function updateContent($contentId, $versionNo, UpdateStruct $struct): Content { $this->logger->logCall(__METHOD__, ['content' => $contentId, 'version' => $versionNo, 'struct' => $struct]); $content = $this->persistenceHandler->contentHandler()->updateContent($contentId, $versionNo, $struct); diff --git a/tests/lib/Persistence/Cache/ContentHandlerTest.php b/tests/lib/Persistence/Cache/ContentHandlerTest.php index 53f13b67ee..537115fb8e 100644 --- a/tests/lib/Persistence/Cache/ContentHandlerTest.php +++ b/tests/lib/Persistence/Cache/ContentHandlerTest.php @@ -10,7 +10,7 @@ use Ibexa\Contracts\Core\Persistence\Content\ContentInfo; use Ibexa\Contracts\Core\Persistence\Content\CreateStruct; use Ibexa\Contracts\Core\Persistence\Content\Handler as SPIContentHandler; -use Ibexa\Contracts\Core\Persistence\Content\Location\Handler as SPILocationHandler; +use Ibexa\Contracts\Core\Persistence\Content\Location\Handler as PersistenceLocationHandler; use Ibexa\Contracts\Core\Persistence\Content\MetadataUpdateStruct; use Ibexa\Contracts\Core\Persistence\Content\Relation; use Ibexa\Contracts\Core\Persistence\Content\Relation as SPIRelation; @@ -444,7 +444,7 @@ public function testUpdateContent(): void $this->loggerMock->expects($this->once())->method('logCall'); $innerContentHandlerMock = $this->createMock(SPIContentHandler::class); - $innerLocationHandlerMock = $this->createMock(SPILocationHandler::class); + $innerLocationHandlerMock = $this->createMock(PersistenceLocationHandler::class); $this->prepareHandlerMocks( $innerContentHandlerMock, @@ -455,26 +455,26 @@ public function testUpdateContent(): void ); $innerContentHandlerMock - ->expects($this->once()) + ->expects(self::once()) ->method('updateContent') ->with(2, 1, new UpdateStruct()) ->willReturn(new Content()); $this->cacheIdentifierGeneratorMock - ->expects($this->exactly(5)) + ->expects(self::exactly(5)) ->method('generateTag') - ->will( - self::returnValueMap([ + ->willReturnMap( + [ ['location', [3], false, 'l-3'], ['location', [4], false, 'l-4'], ['location_path', [3], false, 'lp-3'], ['location_path', [4], false, 'lp-4'], ['content_version', [2, 1], false, 'c-2-v-1'], - ]) + ] ); $this->cacheMock - ->expects($this->once()) + ->expects(self::once()) ->method('invalidateTags') ->with(['l-3', 'l-4', 'lp-3', 'lp-4', 'c-2-v-1']); @@ -482,12 +482,12 @@ public function testUpdateContent(): void $handler->updateContent(2, 1, new UpdateStruct()); } - public function testDeleteContent() + public function testDeleteContent(): void { - $this->loggerMock->expects($this->once())->method('logCall'); + $this->loggerMock->expects(self::once())->method('logCall'); $innerContentHandlerMock = $this->createMock(SPIContentHandler::class); - $innerLocationHandlerMock = $this->createMock(SPILocationHandler::class); + $innerLocationHandlerMock = $this->createMock(PersistenceLocationHandler::class); $this->prepareHandlerMocks( $innerContentHandlerMock, @@ -496,17 +496,17 @@ public function testDeleteContent() ); $innerContentHandlerMock - ->expects($this->once()) + ->expects(self::once()) ->method('deleteContent') ->with(2) ->willReturn(true); $this->cacheMock - ->expects($this->never()) + ->expects(self::never()) ->method('deleteItem'); $this->cacheIdentifierGeneratorMock - ->expects($this->exactly(4)) + ->expects(self::exactly(4)) ->method('generateTag') ->withConsecutive( ['content', [42], false], @@ -517,7 +517,7 @@ public function testDeleteContent() ->willReturnOnConsecutiveCalls('c-42', 'c-2', 'l-3', 'l-4'); $this->cacheMock - ->expects($this->once()) + ->expects(self::once()) ->method('invalidateTags') ->with(['c-42', 'c-2', 'l-3', 'l-4']); @@ -531,19 +531,19 @@ public function testDeleteContent() */ private function prepareHandlerMocks( SPIContentHandler $innerContentHandlerMock, - SPILocationHandler $innerLocationHandlerMock, + PersistenceLocationHandler $innerLocationHandlerMock, int $contentHandlerCount = 1, int $locationHandlerCount = 1, int $loadReverseRelationsCount = 1, int $loadLocationsByContentCount = 1 ): void { $this->persistenceHandlerMock - ->expects($this->exactly($contentHandlerCount)) + ->expects(self::exactly($contentHandlerCount)) ->method('contentHandler') ->willReturn($innerContentHandlerMock); $innerContentHandlerMock - ->expects($this->exactly($loadReverseRelationsCount)) + ->expects(self::exactly($loadReverseRelationsCount)) ->method('loadReverseRelations') ->with(2, APIRelation::FIELD | APIRelation::ASSET) ->willReturn( @@ -553,12 +553,12 @@ private function prepareHandlerMocks( ); $this->persistenceHandlerMock - ->expects($this->exactly($locationHandlerCount)) + ->expects(self::exactly($locationHandlerCount)) ->method('locationHandler') ->willReturn($innerLocationHandlerMock); $innerLocationHandlerMock - ->expects($this->exactly($loadLocationsByContentCount)) + ->expects(self::exactly($loadLocationsByContentCount)) ->method('loadLocationsByContent') ->with(2) ->willReturn( diff --git a/tests/lib/Persistence/Cache/TrashHandlerTest.php b/tests/lib/Persistence/Cache/TrashHandlerTest.php index c88a92b65f..39da8c92c2 100644 --- a/tests/lib/Persistence/Cache/TrashHandlerTest.php +++ b/tests/lib/Persistence/Cache/TrashHandlerTest.php @@ -149,12 +149,12 @@ public function testTrashSubtree() ->willReturn($locationHandlerMock); $this->persistenceHandlerMock - ->expects($this->once()) + ->expects(self::once()) ->method($handlerMethodName) ->willReturn($innerHandler); $contentHandlerMock - ->expects($this->once()) + ->expects(self::once()) ->method('listVersions') ->with($contentId) ->willReturn( @@ -163,13 +163,13 @@ public function testTrashSubtree() ] ); $innerHandler - ->expects($this->once()) + ->expects(self::once()) ->method('trashSubtree') ->with($locationId) ->willReturn(null); $this->cacheIdentifierGeneratorMock - ->expects($this->exactly(4)) + ->expects(self::exactly(4)) ->method('generateTag') ->withConsecutive( ['content_version', [$contentId, $versionNo], false], @@ -180,7 +180,7 @@ public function testTrashSubtree() ->willReturnOnConsecutiveCalls(...$tags); $this->cacheMock - ->expects($this->once()) + ->expects(self::once()) ->method('invalidateTags') ->with($tags); From d46a8f69cd24767f59c02523468378cdf627ac33 Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Mon, 31 Mar 2025 12:27:41 +0200 Subject: [PATCH 08/10] Fixed cs styling and first stan issue --- src/lib/Persistence/Cache/ContentHandler.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/lib/Persistence/Cache/ContentHandler.php b/src/lib/Persistence/Cache/ContentHandler.php index d0de048ede..29764f9a8b 100644 --- a/src/lib/Persistence/Cache/ContentHandler.php +++ b/src/lib/Persistence/Cache/ContentHandler.php @@ -4,7 +4,6 @@ * @copyright Copyright (C) Ibexa AS. All rights reserved. * @license For full copyright and license information view LICENSE file distributed with this source code. */ - namespace Ibexa\Core\Persistence\Cache; use Ibexa\Contracts\Core\Persistence\Content; From e8b66fceb679c7139669d25b77b85b4309df0f9c Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Mon, 31 Mar 2025 13:30:11 +0200 Subject: [PATCH 09/10] Fixed phpstan conflict --- phpstan-baseline.neon | 6 ------ 1 file changed, 6 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 58cf6c53fd..3930bf00ed 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -60516,12 +60516,6 @@ parameters: count: 1 path: tests/lib/Persistence/Cache/ContentHandlerTest.php - - - message: '#^Method Ibexa\\Tests\\Core\\Persistence\\Cache\\ContentHandlerTest\:\:testDeleteContent\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/lib/Persistence/Cache/ContentHandlerTest.php - - message: '#^Method Ibexa\\Tests\\Core\\Persistence\\Cache\\ContentLanguageHandlerTest\:\:providerForCachedLoadMethodsHit\(\) return type has no value type specified in iterable type array\.$#' identifier: missingType.iterableValue From c6eb694b5e54b29957f3b089828a9361494ae221 Mon Sep 17 00:00:00 2001 From: Mateusz Bieniek Date: Mon, 31 Mar 2025 13:44:56 +0200 Subject: [PATCH 10/10] Fixed baseline for 7.4 --- phpstan-baseline-7.4.neon | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/phpstan-baseline-7.4.neon b/phpstan-baseline-7.4.neon index 1b464cbfcd..9b750ce1a2 100644 --- a/phpstan-baseline-7.4.neon +++ b/phpstan-baseline-7.4.neon @@ -216,6 +216,12 @@ parameters: count: 1 path: src/lib/MVC/Symfony/Matcher/ContentBased/UrlAlias.php + - + message: '#^PHPDoc tag @throws with type Ibexa\\Contracts\\Core\\Repository\\Exceptions\\InvalidArgumentException\|Psr\\Cache\\InvalidArgumentException is not subtype of Throwable$#' + identifier: throws.notThrowable + count: 1 + path: src/lib/Persistence/Cache/ContentHandler.php + - message: '#^PHPDoc tag @throws with type Psr\\Cache\\InvalidArgumentException is not subtype of Throwable$#' identifier: throws.notThrowable