From dfc63897f377622e0fc7a75d33a3e34fcc882121 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Wed, 11 Oct 2023 18:27:23 +0200 Subject: [PATCH 1/4] IBX-6631: Enriched `TrashItem` with `removedLocationContentIdMap` --- .../Repository/Values/Content/TrashItem.php | 11 ++++++++++ .../Core/Persistence/Cache/TrashHandler.php | 8 +++---- .../Legacy/Content/Location/Mapper.php | 2 +- .../Legacy/Content/Location/Trash/Handler.php | 22 ++++++++----------- eZ/Publish/Core/Repository/TrashService.php | 5 ++++- .../Content/Location/Trash/Handler.php | 8 +++---- .../Persistence/Content/Location/Trashed.php | 3 +++ 7 files changed, 35 insertions(+), 24 deletions(-) diff --git a/eZ/Publish/API/Repository/Values/Content/TrashItem.php b/eZ/Publish/API/Repository/Values/Content/TrashItem.php index 009ab9d81e..6f0545d23d 100644 --- a/eZ/Publish/API/Repository/Values/Content/TrashItem.php +++ b/eZ/Publish/API/Repository/Values/Content/TrashItem.php @@ -19,4 +19,15 @@ abstract class TrashItem extends Location * @var \DateTime */ protected $trashed; + + /** @var array */ + protected $removedLocationContentIdMap = []; + + /** + * @return array + */ + public function getRemovedLocationContentIdMap(): array + { + return $this->removedLocationContentIdMap; + } } diff --git a/eZ/Publish/Core/Persistence/Cache/TrashHandler.php b/eZ/Publish/Core/Persistence/Cache/TrashHandler.php index 78e9e0eb0c..db9baee5e8 100644 --- a/eZ/Publish/Core/Persistence/Cache/TrashHandler.php +++ b/eZ/Publish/Core/Persistence/Cache/TrashHandler.php @@ -7,6 +7,7 @@ namespace eZ\Publish\Core\Persistence\Cache; use eZ\Publish\API\Repository\Values\Content\Query\Criterion; +use eZ\Publish\SPI\Persistence\Content\Location; use eZ\Publish\SPI\Persistence\Content\Location\Trash\Handler as TrashHandlerInterface; use eZ\Publish\SPI\Persistence\Content\Relation; @@ -19,14 +20,11 @@ class TrashHandler extends AbstractHandler implements TrashHandlerInterface private const CONTENT_IDENTIFIER = 'content'; private const LOCATION_PATH_IDENTIFIER = 'location_path'; - /** - * {@inheritdoc} - */ - public function loadTrashItem($id) + public function loadTrashItem(int $id, array $trashedLocationsContentMap = []): Location\Trashed { $this->logger->logCall(__METHOD__, ['id' => $id]); - return $this->persistenceHandler->trashHandler()->loadTrashItem($id); + return $this->persistenceHandler->trashHandler()->loadTrashItem($id, $trashedLocationsContentMap); } /** diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php index b9d213c4a4..f45903c26a 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php @@ -25,7 +25,7 @@ class Mapper * @param string $prefix * @param \eZ\Publish\SPI\Persistence\Content\Location|null $location * - * @return \eZ\Publish\SPI\Persistence\Content\Location + * @return \eZ\Publish\SPI\Persistence\Content\Location|\eZ\Publish\SPI\Persistence\Content\Location\Trashed */ public function createLocationFromRow(array $data, $prefix = '', ?Location $location = null) { 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 c395782956..ac974ac1fe 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php @@ -74,21 +74,15 @@ public function __construct( $this->contentHandler = $contentHandler; } - /** - * Loads the data for the trashed location identified by $id. - * $id is the same as original location (which has been previously trashed). - * - * @param int $id - * - * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException - * - * @return \eZ\Publish\SPI\Persistence\Content\Location\Trashed - */ - public function loadTrashItem($id) + public function loadTrashItem(int $id, array $trashedLocationsContentMap = []): Trashed { $data = $this->locationGateway->loadTrashByLocation($id); - return $this->locationMapper->createLocationFromRow($data, null, new Trashed()); + return $this->locationMapper->createLocationFromRow( + $data, + null, + new Trashed(['removedLocationContentIdMap' => $trashedLocationsContentMap]), + ); } /** @@ -109,6 +103,7 @@ public function trashSubtree($locationId) $locationRows = $this->locationGateway->getSubtreeContent($locationId); $isLocationRemoved = false; $parentLocationId = null; + $trashedLocationsContentMap = []; foreach ($locationRows as $locationRow) { if ($locationRow['node_id'] == $locationId) { @@ -117,6 +112,7 @@ public function trashSubtree($locationId) if ($this->locationGateway->countLocationsByContentId($locationRow['contentobject_id']) == 1) { $this->locationGateway->trashLocation($locationRow['node_id']); + $trashedLocationsContentMap[$locationRow['node_id']] = $locationRow['contentobject_id']; } else { if ($locationRow['node_id'] == $locationId) { $isLocationRemoved = true; @@ -143,7 +139,7 @@ public function trashSubtree($locationId) $this->locationHandler->markSubtreeModified($parentLocationId, time()); } - return $isLocationRemoved ? null : $this->loadTrashItem($locationId); + return $isLocationRemoved ? null : $this->loadTrashItem($locationId, $trashedLocationsContentMap); } /** diff --git a/eZ/Publish/Core/Repository/TrashService.php b/eZ/Publish/Core/Repository/TrashService.php index 55c628d3ff..3e3e31de7f 100644 --- a/eZ/Publish/Core/Repository/TrashService.php +++ b/eZ/Publish/Core/Repository/TrashService.php @@ -377,7 +377,10 @@ protected function buildDomainTrashItemObject(Trashed $spiTrashItem, Content $co 'depth' => $spiTrashItem->depth, 'sortField' => $spiTrashItem->sortField, 'sortOrder' => $spiTrashItem->sortOrder, - 'trashed' => isset($spiTrashItem->trashed) ? new DateTime('@' . $spiTrashItem->trashed) : new DateTime('@0'), + 'trashed' => isset($spiTrashItem->trashed) + ? new DateTime('@' . $spiTrashItem->trashed) + : new DateTime('@0'), + 'removedLocationContentIdMap' => $spiTrashItem->removedLocationContentIdMap, 'parentLocation' => $this->proxyDomainMapper->createLocationProxy($spiTrashItem->parentId), ] ); diff --git a/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php b/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php index 4eda8ce721..2385d3b74b 100644 --- a/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php +++ b/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php @@ -7,6 +7,8 @@ namespace eZ\Publish\SPI\Persistence\Content\Location\Trash; use eZ\Publish\API\Repository\Values\Content\Query\Criterion; +use eZ\Publish\SPI\Persistence\Content\Location; +use eZ\Publish\SPI\Persistence\Content\Location\Trashed; /** * The Trash Handler interface defines operations on Location elements in the storage engine. @@ -17,13 +19,11 @@ interface Handler * Loads the data for the trashed location identified by $id. * $id is the same as original location (which has been previously trashed). * - * @param int $id + * @param array $trashedLocationsContentMap * * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException - * - * @return \eZ\Publish\SPI\Persistence\Content\Location\Trashed */ - public function loadTrashItem($id); + public function loadTrashItem(int $id, array $trashedLocationsContentMap = []): Trashed; /** * Sends a subtree starting to $locationId to the trash diff --git a/eZ/Publish/SPI/Persistence/Content/Location/Trashed.php b/eZ/Publish/SPI/Persistence/Content/Location/Trashed.php index a6ff294dcd..3cae5d9ac7 100644 --- a/eZ/Publish/SPI/Persistence/Content/Location/Trashed.php +++ b/eZ/Publish/SPI/Persistence/Content/Location/Trashed.php @@ -19,4 +19,7 @@ class Trashed extends Location * @var mixed Trashed timestamp. */ public $trashed; + + /** @var array Location ID to a Content ID map of removed items */ + public $removedLocationContentIdMap = []; } From 94b77034f595911046f2f236e7f04ce1d3e5fd28 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Thu, 12 Oct 2023 10:54:10 +0200 Subject: [PATCH 2/4] IBX-6631: Simplified the solution --- .../Core/Persistence/Cache/TrashHandler.php | 8 +++--- .../Legacy/Content/Location/Mapper.php | 2 +- .../Legacy/Content/Location/Trash/Handler.php | 27 ++++++++++++++----- .../Content/Location/Trash/Handler.php | 8 +++--- 4 files changed, 30 insertions(+), 15 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Cache/TrashHandler.php b/eZ/Publish/Core/Persistence/Cache/TrashHandler.php index db9baee5e8..78e9e0eb0c 100644 --- a/eZ/Publish/Core/Persistence/Cache/TrashHandler.php +++ b/eZ/Publish/Core/Persistence/Cache/TrashHandler.php @@ -7,7 +7,6 @@ namespace eZ\Publish\Core\Persistence\Cache; use eZ\Publish\API\Repository\Values\Content\Query\Criterion; -use eZ\Publish\SPI\Persistence\Content\Location; use eZ\Publish\SPI\Persistence\Content\Location\Trash\Handler as TrashHandlerInterface; use eZ\Publish\SPI\Persistence\Content\Relation; @@ -20,11 +19,14 @@ class TrashHandler extends AbstractHandler implements TrashHandlerInterface private const CONTENT_IDENTIFIER = 'content'; private const LOCATION_PATH_IDENTIFIER = 'location_path'; - public function loadTrashItem(int $id, array $trashedLocationsContentMap = []): Location\Trashed + /** + * {@inheritdoc} + */ + public function loadTrashItem($id) { $this->logger->logCall(__METHOD__, ['id' => $id]); - return $this->persistenceHandler->trashHandler()->loadTrashItem($id, $trashedLocationsContentMap); + return $this->persistenceHandler->trashHandler()->loadTrashItem($id); } /** diff --git a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php index f45903c26a..b9d213c4a4 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Mapper.php @@ -25,7 +25,7 @@ class Mapper * @param string $prefix * @param \eZ\Publish\SPI\Persistence\Content\Location|null $location * - * @return \eZ\Publish\SPI\Persistence\Content\Location|\eZ\Publish\SPI\Persistence\Content\Location\Trashed + * @return \eZ\Publish\SPI\Persistence\Content\Location */ public function createLocationFromRow(array $data, $prefix = '', ?Location $location = null) { 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 ac974ac1fe..07fe1276df 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php @@ -74,15 +74,21 @@ public function __construct( $this->contentHandler = $contentHandler; } - public function loadTrashItem(int $id, array $trashedLocationsContentMap = []): Trashed + /** + * Loads the data for the trashed location identified by $id. + * $id is the same as original location (which has been previously trashed). + * + * @param int $id + * + * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException + * + * @return \eZ\Publish\SPI\Persistence\Content\Location\Trashed + */ + public function loadTrashItem($id) { $data = $this->locationGateway->loadTrashByLocation($id); - return $this->locationMapper->createLocationFromRow( - $data, - null, - new Trashed(['removedLocationContentIdMap' => $trashedLocationsContentMap]), - ); + return $this->locationMapper->createLocationFromRow($data, null, new Trashed()); } /** @@ -139,7 +145,14 @@ public function trashSubtree($locationId) $this->locationHandler->markSubtreeModified($parentLocationId, time()); } - return $isLocationRemoved ? null : $this->loadTrashItem($locationId, $trashedLocationsContentMap); + if ($isLocationRemoved === true) { + return null; + } + + $trashItem = $this->loadTrashItem($locationId); + $trashItem->removedLocationContentIdMap = $trashedLocationsContentMap; + + return $trashItem; } /** diff --git a/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php b/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php index 2385d3b74b..4eda8ce721 100644 --- a/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php +++ b/eZ/Publish/SPI/Persistence/Content/Location/Trash/Handler.php @@ -7,8 +7,6 @@ namespace eZ\Publish\SPI\Persistence\Content\Location\Trash; use eZ\Publish\API\Repository\Values\Content\Query\Criterion; -use eZ\Publish\SPI\Persistence\Content\Location; -use eZ\Publish\SPI\Persistence\Content\Location\Trashed; /** * The Trash Handler interface defines operations on Location elements in the storage engine. @@ -19,11 +17,13 @@ interface Handler * Loads the data for the trashed location identified by $id. * $id is the same as original location (which has been previously trashed). * - * @param array $trashedLocationsContentMap + * @param int $id * * @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException + * + * @return \eZ\Publish\SPI\Persistence\Content\Location\Trashed */ - public function loadTrashItem(int $id, array $trashedLocationsContentMap = []): Trashed; + public function loadTrashItem($id); /** * Sends a subtree starting to $locationId to the trash From 8c7f28fed9d2d035de777ec65e1b219ba50c79aa Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Thu, 12 Oct 2023 15:51:14 +0200 Subject: [PATCH 3/4] IBX-6631: Removed unnecessary assert --- eZ/Publish/API/Repository/Tests/TrashServiceTest.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/TrashServiceTest.php b/eZ/Publish/API/Repository/Tests/TrashServiceTest.php index c46d00ae25..4a1f3ca67c 100644 --- a/eZ/Publish/API/Repository/Tests/TrashServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/TrashServiceTest.php @@ -254,11 +254,6 @@ public function testLoadTrashItem() $trashItemReloaded->pathString ); - $this->assertEquals( - $trashItem, - $trashItemReloaded - ); - $this->assertInstanceOf( DateTime::class, $trashItemReloaded->trashed From 48730c11ce932de1ae3d0c6173b299d1c56ce353 Mon Sep 17 00:00:00 2001 From: Bartek Wajda Date: Fri, 13 Oct 2023 14:22:42 +0200 Subject: [PATCH 4/4] IBX-6631: Added an integration test --- .../API/Repository/Tests/TrashServiceTest.php | 25 +++++++++++++++++++ .../Legacy/Content/Location/Trash/Handler.php | 6 ++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/TrashServiceTest.php b/eZ/Publish/API/Repository/Tests/TrashServiceTest.php index 4a1f3ca67c..c4bbe2950a 100644 --- a/eZ/Publish/API/Repository/Tests/TrashServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/TrashServiceTest.php @@ -1036,6 +1036,31 @@ public function testDeleteThrowsNotFoundExceptionForNonExistingTrashItem() )); } + public function testTrashProperlyAssignsRemovedLocationContentMapToTrashItem(): void + { + $repository = $this->getRepository(); + $trashService = $repository->getTrashService(); + $locationService = $repository->getLocationService(); + + $folder1 = $this->createFolder(['eng-GB' => 'Folder1'], 2); + $folder2 = $this->createFolder(['eng-GB' => 'Folder2'], $folder1->contentInfo->getMainLocationId()); + $folder3 = $this->createFolder(['eng-GB' => 'Folder2'], $folder2->contentInfo->getMainLocationId()); + + $folderLocation = $locationService->loadLocation($folder1->contentInfo->getMainLocationId()); + + $trashItem = $trashService->trash($folderLocation); + $removedLocationContentMap = $trashItem->getRemovedLocationContentIdMap(); + + self::assertSame( + [ + $folderLocation->id => $folder1->id, + $folder2->contentInfo->getMainLocationId() => $folder2->id, + $folder3->contentInfo->getMainLocationId() => $folder3->id, + ], + $removedLocationContentMap, + ); + } + /** * @return array */ 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 07fe1276df..d4affc5cdd 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Content/Location/Trash/Handler.php @@ -109,7 +109,7 @@ public function trashSubtree($locationId) $locationRows = $this->locationGateway->getSubtreeContent($locationId); $isLocationRemoved = false; $parentLocationId = null; - $trashedLocationsContentMap = []; + $removedLocationsContentMap = []; foreach ($locationRows as $locationRow) { if ($locationRow['node_id'] == $locationId) { @@ -118,7 +118,7 @@ public function trashSubtree($locationId) if ($this->locationGateway->countLocationsByContentId($locationRow['contentobject_id']) == 1) { $this->locationGateway->trashLocation($locationRow['node_id']); - $trashedLocationsContentMap[$locationRow['node_id']] = $locationRow['contentobject_id']; + $removedLocationsContentMap[(int)$locationRow['node_id']] = (int)$locationRow['contentobject_id']; } else { if ($locationRow['node_id'] == $locationId) { $isLocationRemoved = true; @@ -150,7 +150,7 @@ public function trashSubtree($locationId) } $trashItem = $this->loadTrashItem($locationId); - $trashItem->removedLocationContentIdMap = $trashedLocationsContentMap; + $trashItem->removedLocationContentIdMap = $removedLocationsContentMap; return $trashItem; }