From 3cae2774f038c4c4984944475a2e27989134d4ea Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Wed, 13 Dec 2023 17:41:21 +0100 Subject: [PATCH 1/3] IBX-7346: Reindexed reverse-related content after deleting source content --- .../Content/Trash/TrashItemDeleteResult.php | 3 + .../Legacy/Content/Location/Trash/Handler.php | 3 + .../Content/Location/TrashHandlerTest.php | 110 +++++++++--------- .../EventSubscriber/TrashEventSubscriber.php | 21 ++++ phpstan-baseline.neon | 10 -- .../TrashEventSubscriberTest.php | 77 ++++++++++++ 6 files changed, 162 insertions(+), 62 deletions(-) create mode 100644 tests/lib/Search/Common/EventSubscriber/TrashEventSubscriberTest.php diff --git a/eZ/Publish/API/Repository/Values/Content/Trash/TrashItemDeleteResult.php b/eZ/Publish/API/Repository/Values/Content/Trash/TrashItemDeleteResult.php index 3a40485349..96d95ab596 100644 --- a/eZ/Publish/API/Repository/Values/Content/Trash/TrashItemDeleteResult.php +++ b/eZ/Publish/API/Repository/Values/Content/Trash/TrashItemDeleteResult.php @@ -24,4 +24,7 @@ class TrashItemDeleteResult extends ValueObject * @var bool */ public $contentRemoved = false; + + /** @var array */ + public $reverseRelationContentIds = []; } diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php index d4affc5cdd..0a347b6b3a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php @@ -246,11 +246,14 @@ protected function delete(Trashed $trashItem) $result->trashItemId = $trashItem->id; $result->contentId = $trashItem->contentId; + $reverseRelations = $this->contentHandler->loadReverseRelations($trashItem->contentId); + $this->locationGateway->removeElementFromTrash($trashItem->id); if ($this->locationGateway->countLocationsByContentId($trashItem->contentId) < 1) { $this->contentHandler->deleteContent($trashItem->contentId); $result->contentRemoved = true; + $result->reverseRelationContentIds = array_column($reverseRelations, 'sourceContentId'); } return $result; diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Location/TrashHandlerTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Location/TrashHandlerTest.php index 92a64d17a0..ae7d8d57c7 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Location/TrashHandlerTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Content/Location/TrashHandlerTest.php @@ -329,7 +329,7 @@ public function testLoadTrashItem() /** * @covers \eZ\Publish\Core\Persistence\Legacy\Content\Location\Trash\Handler::emptyTrash */ - public function testEmptyTrash() + public function testEmptyTrash(): void { $handler = $this->getTrashHandler(); @@ -354,48 +354,51 @@ public function testEmptyTrash() $iLocation = 0; $this->locationGateway - ->expects($this->at($i++)) + ->expects(self::at($i++)) ->method('countTrashed') ->willReturn(2); $this->locationGateway - ->expects($this->at($i++)) + ->expects(self::at($i++)) ->method('listTrashed') - ->will( - $this->returnValue($expectedTrashed) - ); + ->willReturn($expectedTrashed); $trashedItemIds = []; $trashedContentIds = []; foreach ($expectedTrashed as $trashedElement) { $this->locationMapper - ->expects($this->at($iLocation++)) + ->expects(self::at($iLocation++)) ->method('createLocationFromRow') - ->will( - $this->returnValue( - new Trashed( - [ - 'id' => $trashedElement['node_id'], - 'contentId' => $trashedElement['contentobject_id'], - 'pathString' => $trashedElement['path_string'], - ] - ) + ->willReturn( + new Trashed( + [ + 'id' => $trashedElement['node_id'], + 'contentId' => $trashedElement['contentobject_id'], + 'pathString' => $trashedElement['path_string'], + ] ) ); + + $this->contentHandler + ->expects(self::at($iContent++)) + ->method('loadReverseRelations') + ->with($trashedElement['contentobject_id']) + ->willReturn([]); + $this->locationGateway - ->expects($this->at($i++)) + ->expects(self::at($i++)) ->method('removeElementFromTrash') ->with($trashedElement['node_id']); $this->locationGateway - ->expects($this->at($i++)) + ->expects(self::at($i++)) ->method('countLocationsByContentId') ->with($trashedElement['contentobject_id']) - ->will($this->returnValue(0)); + ->willReturn(0); $this->contentHandler - ->expects($this->at($iContent++)) + ->expects(self::at($iContent++)) ->method('deleteContent') ->with($trashedElement['contentobject_id']); @@ -405,75 +408,78 @@ public function testEmptyTrash() $returnValue = $handler->emptyTrash(); - $this->assertInstanceOf(TrashItemDeleteResultList::class, $returnValue); + self::assertInstanceOf(TrashItemDeleteResultList::class, $returnValue); foreach ($returnValue->items as $key => $trashItemDeleteResult) { - $this->assertEquals($trashItemDeleteResult->trashItemId, $trashedItemIds[$key]); - $this->assertEquals($trashItemDeleteResult->contentId, $trashedContentIds[$key]); - $this->assertTrue($trashItemDeleteResult->contentRemoved); + self::assertEquals($trashItemDeleteResult->trashItemId, $trashedItemIds[$key]); + self::assertEquals($trashItemDeleteResult->contentId, $trashedContentIds[$key]); + self::assertTrue($trashItemDeleteResult->contentRemoved); } } /** * @covers \eZ\Publish\Core\Persistence\Legacy\Content\Location\Trash\Handler::deleteTrashItem */ - public function testDeleteTrashItemNoMoreLocations() + public function testDeleteTrashItemNoMoreLocations(): void { $handler = $this->getTrashHandler(); $trashItemId = 69; $contentId = 67; + $this->locationGateway - ->expects($this->once()) + ->expects(self::once()) ->method('loadTrashByLocation') ->with($trashItemId) - ->will( - $this->returnValue( - [ - 'node_id' => $trashItemId, - 'contentobject_id' => $contentId, - 'path_string' => '/1/2/69', - ] - ) + ->willReturn( + [ + 'node_id' => $trashItemId, + 'contentobject_id' => $contentId, + 'path_string' => '/1/2/69', + ] ); $this->locationMapper - ->expects($this->once()) + ->expects(self::once()) ->method('createLocationFromRow') - ->will( - $this->returnValue( - new Trashed( - [ - 'id' => $trashItemId, - 'contentId' => $contentId, - 'pathString' => '/1/2/69', - ] - ) + ->willReturn( + new Trashed( + [ + 'id' => $trashItemId, + 'contentId' => $contentId, + 'pathString' => '/1/2/69', + ] ) ); + $this->contentHandler + ->expects(self::once()) + ->method('loadReverseRelations') + ->with($contentId) + ->willReturn([]); + $this->locationGateway - ->expects($this->once()) + ->expects(self::once()) ->method('removeElementFromTrash') ->with($trashItemId); $this->locationGateway - ->expects($this->once()) + ->expects(self::once()) ->method('countLocationsByContentId') ->with($contentId) - ->will($this->returnValue(0)); + ->willReturn(0); $this->contentHandler - ->expects($this->once()) + ->expects(self::once()) ->method('deleteContent') ->with($contentId); $trashItemDeleteResult = $handler->deleteTrashItem($trashItemId); - $this->assertInstanceOf(TrashItemDeleteResult::class, $trashItemDeleteResult); - $this->assertEquals($trashItemId, $trashItemDeleteResult->trashItemId); - $this->assertEquals($contentId, $trashItemDeleteResult->contentId); - $this->assertTrue($trashItemDeleteResult->contentRemoved); + self::assertInstanceOf(TrashItemDeleteResult::class, $trashItemDeleteResult); + self::assertEquals($trashItemId, $trashItemDeleteResult->trashItemId); + self::assertEquals($contentId, $trashItemDeleteResult->contentId); + self::assertTrue($trashItemDeleteResult->contentRemoved); } /** diff --git a/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php b/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php index f6866c360d..545a3028a0 100644 --- a/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php +++ b/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php @@ -6,18 +6,27 @@ */ namespace eZ\Publish\Core\Search\Common\EventSubscriber; +use eZ\Publish\API\Repository\Events\Trash\DeleteTrashItemEvent; use eZ\Publish\API\Repository\Events\Trash\RecoverEvent; use eZ\Publish\API\Repository\Events\Trash\TrashEvent; use eZ\Publish\API\Repository\Values\Content\TrashItem; +use eZ\Publish\SPI\Persistence\Handler as PersistenceHandler; +use eZ\Publish\SPI\Search\Handler as SearchHandler; use Symfony\Component\EventDispatcher\EventSubscriberInterface; class TrashEventSubscriber extends AbstractSearchEventSubscriber implements EventSubscriberInterface { + public function __construct(SearchHandler $searchHandler, PersistenceHandler $persistenceHandler) + { + parent::__construct($searchHandler, $persistenceHandler); + } + public static function getSubscribedEvents(): array { return [ RecoverEvent::class => 'onRecover', TrashEvent::class => 'onTrash', + DeleteTrashItemEvent::class => 'onDeleteTrashItem', ]; } @@ -39,4 +48,16 @@ public function onTrash(TrashEvent $event) $event->getLocation()->contentId ); } + + public function onDeleteTrashItem(DeleteTrashItemEvent $event): void + { + $contentHandler = $this->persistenceHandler->contentHandler(); + + $reverseRelationContentIds = $event->getResult()->reverseRelationContentIds; + foreach ($reverseRelationContentIds as $contentId) { + $persistenceContent = $contentHandler->load($contentId); + + $this->searchHandler->indexContent($persistenceContent); + } + } } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 985db858f0..0803572c0d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -280,11 +280,6 @@ parameters: count: 3 path: eZ/Publish/API/Repository/Tests/SearchServiceTranslationLanguageFallbackTest.php - - - message: "#^Undefined variable\\: \\$loader$#" - count: 3 - path: eZ/Publish/API/Repository/Tests/SetupFactory/Legacy.php - - message: "#^Method eZ\\\\Publish\\\\API\\\\Repository\\\\Tests\\\\URLWildcardServiceAuthorizationTest\\:\\:testCreateThrowsUnauthorizedException\\(\\) should return eZ\\\\Publish\\\\API\\\\Repository\\\\Values\\\\Content\\\\URLWildcard but return statement is missing\\.$#" count: 1 @@ -505,11 +500,6 @@ parameters: count: 1 path: eZ/Publish/Core/Persistence/Legacy/Content/Type/Handler.php - - - message: "#^Undefined variable\\: \\$loader$#" - count: 2 - path: eZ/Publish/Core/Persistence/Legacy/Tests/HandlerTest.php - - message: "#^Class eZ\\\\Publish\\\\SPI\\\\Persistence\\\\Content\\\\UrlAlias referenced with incorrect case\\: eZ\\\\Publish\\\\SPI\\\\Persistence\\\\Content\\\\URLAlias\\.$#" count: 2 diff --git a/tests/lib/Search/Common/EventSubscriber/TrashEventSubscriberTest.php b/tests/lib/Search/Common/EventSubscriber/TrashEventSubscriberTest.php new file mode 100644 index 0000000000..1dbb291d71 --- /dev/null +++ b/tests/lib/Search/Common/EventSubscriber/TrashEventSubscriberTest.php @@ -0,0 +1,77 @@ +searchHandler = $this->createMock(SearchHandler::class); + $this->persistenceHandler = $this->createMock(PersistenceHandler::class); + + $this->subscriber = new TrashEventSubscriber( + $this->searchHandler, + $this->persistenceHandler + ); + } + + public function testOnDeleteTrashItem(): void + { + $trashItem = new TrashItem(['id' => 12345]); + $reverseRelationContentId = 12; + $trashItemDeleteResult = new TrashItemDeleteResult( + [ + 'trashItemId' => $trashItem->id, + 'reverseRelationContentIds' => [$reverseRelationContentId], + ] + ); + + $this->persistenceHandler + ->expects(self::once()) + ->method('contentHandler') + ->willReturn($contentHandler = $this->createMock(Handler::class)); + + $contentHandler + ->expects(self::once()) + ->method('load') + ->with($reverseRelationContentId) + ->willReturn($content = new PersistenceContent()); + + $this->searchHandler + ->expects(self::once()) + ->method('indexContent') + ->with($content); + + $this->subscriber->onDeleteTrashItem( + new DeleteTrashItemEvent( + $trashItemDeleteResult, + $trashItem, + ) + ); + } +} From 1bfc464f38018093bbe726f06dba7e2d38dfe989 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Wed, 13 Dec 2023 17:56:01 +0100 Subject: [PATCH 2/3] IBX-7346: Handled trash emptying --- .../EventSubscriber/TrashEventSubscriber.php | 24 +++++++++++++++---- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php b/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php index 545a3028a0..ea5c2d53c1 100644 --- a/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php +++ b/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php @@ -7,6 +7,7 @@ namespace eZ\Publish\Core\Search\Common\EventSubscriber; use eZ\Publish\API\Repository\Events\Trash\DeleteTrashItemEvent; +use eZ\Publish\API\Repository\Events\Trash\EmptyTrashEvent; use eZ\Publish\API\Repository\Events\Trash\RecoverEvent; use eZ\Publish\API\Repository\Events\Trash\TrashEvent; use eZ\Publish\API\Repository\Values\Content\TrashItem; @@ -16,17 +17,13 @@ class TrashEventSubscriber extends AbstractSearchEventSubscriber implements EventSubscriberInterface { - public function __construct(SearchHandler $searchHandler, PersistenceHandler $persistenceHandler) - { - parent::__construct($searchHandler, $persistenceHandler); - } - public static function getSubscribedEvents(): array { return [ RecoverEvent::class => 'onRecover', TrashEvent::class => 'onTrash', DeleteTrashItemEvent::class => 'onDeleteTrashItem', + EmptyTrashEvent::class => 'onEmptyTrashEvent', ]; } @@ -60,4 +57,21 @@ public function onDeleteTrashItem(DeleteTrashItemEvent $event): void $this->searchHandler->indexContent($persistenceContent); } } + + public function onEmptyTrashEvent(EmptyTrashEvent $event): void + { + $contentHandler = $this->persistenceHandler->contentHandler(); + + $results = $event->getResultList()->getIterator(); + + /** @var \eZ\Publish\API\Repository\Values\Content\Trash\TrashItemDeleteResult $result */ + foreach ($results as $result) { + $reverseRelationContentIds = $result->reverseRelationContentIds; + foreach ($reverseRelationContentIds as $contentId) { + $persistenceContent = $contentHandler->load($contentId); + + $this->searchHandler->indexContent($persistenceContent); + } + } + } } From b6cbf20fca1754973ff953350cbeec47be9aa5c1 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Wed, 13 Dec 2023 18:12:07 +0100 Subject: [PATCH 3/3] IBX-7346: CS --- .../Core/Search/Common/EventSubscriber/TrashEventSubscriber.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php b/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php index ea5c2d53c1..a41cd99c3c 100644 --- a/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php +++ b/eZ/Publish/Core/Search/Common/EventSubscriber/TrashEventSubscriber.php @@ -11,8 +11,6 @@ use eZ\Publish\API\Repository\Events\Trash\RecoverEvent; use eZ\Publish\API\Repository\Events\Trash\TrashEvent; use eZ\Publish\API\Repository\Values\Content\TrashItem; -use eZ\Publish\SPI\Persistence\Handler as PersistenceHandler; -use eZ\Publish\SPI\Search\Handler as SearchHandler; use Symfony\Component\EventDispatcher\EventSubscriberInterface; class TrashEventSubscriber extends AbstractSearchEventSubscriber implements EventSubscriberInterface