From ddca556c2a0cd312bf69a4bdd20a3db0e037c043 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Fri, 24 May 2024 15:46:47 +0200 Subject: [PATCH 01/13] IBX-6773: Bookmarks for non-accessible contents cause exception --- .../Repository/Tests/LocationServiceTest.php | 19 ++++++-- .../Content/Query/Criterion/Bookmark.php | 43 +++++++++++++++++ .../Content/Query/SortClause/BookmarkId.php | 29 ++++++++++++ .../Persistence/Cache/BookmarkHandler.php | 4 ++ .../Persistence/Legacy/Bookmark/Gateway.php | 4 ++ .../Bookmark/Gateway/DoctrineDatabase.php | 4 ++ .../Bookmark/Gateway/ExceptionConversion.php | 16 +++++++ .../Persistence/Legacy/Bookmark/Handler.php | 4 ++ .../Location/BookmarkQueryBuilder.php | 47 +++++++++++++++++++ .../Bookmark/IdSortClauseQueryBuilder.php | 31 ++++++++++++ .../Location/BookmarkQueryBuilderTest.php | 45 ++++++++++++++++++ .../Core/Repository/BookmarkService.php | 27 +++++++---- .../Tests/Service/Mock/BookmarkTest.php | 45 ++---------------- .../SPI/Persistence/Bookmark/Handler.php | 4 ++ 14 files changed, 270 insertions(+), 52 deletions(-) create mode 100644 eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php create mode 100644 eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php create mode 100644 eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php create mode 100644 eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php create mode 100644 eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php diff --git a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php index 153f768ba7..4238ae4671 100644 --- a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php @@ -1965,6 +1965,7 @@ public function testBookmarksAreSwappedAfterSwapLocation() $mediaLocationId = $this->generateId('location', 43); $demoDesignLocationId = $this->generateId('location', 56); + $contactUsLocationId = $this->generateId('location', 60); /* BEGIN: Use Case */ $locationService = $repository->getLocationService(); @@ -1972,6 +1973,7 @@ public function testBookmarksAreSwappedAfterSwapLocation() $mediaLocation = $locationService->loadLocation($mediaLocationId); $demoDesignLocation = $locationService->loadLocation($demoDesignLocationId); + $contactUsLocation = $locationService->loadLocation($contactUsLocationId); // Bookmark locations $bookmarkService->createBookmark($mediaLocation); @@ -1980,13 +1982,24 @@ public function testBookmarksAreSwappedAfterSwapLocation() $beforeSwap = $bookmarkService->loadBookmarks(); // Swaps the content referred to by the locations - $locationService->swapLocation($mediaLocation, $demoDesignLocation); + $locationService->swapLocation($demoDesignLocation, $contactUsLocation); $afterSwap = $bookmarkService->loadBookmarks(); /* END: Use Case */ - $this->assertEquals($beforeSwap->items[0]->id, $afterSwap->items[1]->id); - $this->assertEquals($beforeSwap->items[1]->id, $afterSwap->items[0]->id); + $expectedIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) { + if ($item->id === $demoDesignLocationId) { + return $contactUsLocationId; + } + + return $item->id; + }, $beforeSwap->items); + + $actualIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) { + return $item->id; + }, $afterSwap->items); + + $this->assertEquals($expectedIdsAfter, $actualIdsAfter); } /** diff --git a/eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php b/eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php new file mode 100644 index 0000000000..faf57568af --- /dev/null +++ b/eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php @@ -0,0 +1,43 @@ +joinOnce( + 'location', + DoctrineDatabase::TABLE_BOOKMARKS, + 'bookmark', + 'location.node_id = bookmark.node_id' + ); + + return $queryBuilder->expr()->eq( + 'bookmark.user_id', + $queryBuilder->createNamedParameter( + (int)$criterion->value[0], + ParameterType::INTEGER + ) + ); + } +} diff --git a/eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php b/eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php new file mode 100644 index 0000000000..ac2c8ea4e0 --- /dev/null +++ b/eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php @@ -0,0 +1,31 @@ +addSelect('bookmark.id'); + $queryBuilder->addOrderBy('bookmark.id', $sortClause->direction); + } +} diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php new file mode 100644 index 0000000000..eb3e52bae0 --- /dev/null +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -0,0 +1,45 @@ + [ + new Criterion\Bookmark(14), + 'bookmark.user_id = :dcValue1', + ['dcValue1' => 14], + ]; + + yield 'Bookmarks locations for user_id=14 OR user_id=7' => [ + new Criterion\LogicalOr( + [ + new Criterion\Bookmark(14), + new Criterion\Bookmark(7), + ] + ), + '(bookmark.user_id = :dcValue1) OR (bookmark.user_id = :dcValue2)', + ['dcValue1' => 14, 'dcValue2' => 7], + ]; + } + + protected function getCriterionQueryBuilders(): iterable + { + return [new BookmarkQueryBuilder()]; + } +} diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index 05d5d08cde..bf9914289f 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -13,8 +13,11 @@ use eZ\Publish\API\Repository\Repository as RepositoryInterface; use eZ\Publish\API\Repository\Values\Bookmark\BookmarkList; use eZ\Publish\API\Repository\Values\Content\Location; +use eZ\Publish\API\Repository\Values\Content\Query; +use eZ\Publish\API\Repository\Values\Content\Query\Criterion; +use eZ\Publish\API\Repository\Values\Content\Query\SortClause; +use eZ\Publish\API\Repository\Values\Filter\Filter; use eZ\Publish\Core\Base\Exceptions\InvalidArgumentException; -use eZ\Publish\SPI\Persistence\Bookmark\Bookmark; use eZ\Publish\SPI\Persistence\Bookmark\CreateStruct; use eZ\Publish\SPI\Persistence\Bookmark\Handler as BookmarkHandler; @@ -97,16 +100,22 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList { $currentUserId = $this->getCurrentUserId(); - $list = new BookmarkList(); - $list->totalCount = $this->bookmarkHandler->countUserBookmarks($currentUserId); - if ($list->totalCount > 0) { - $bookmarks = $this->bookmarkHandler->loadUserBookmarks($currentUserId, $offset, $limit); - - $list->items = array_map(function (Bookmark $bookmark) { - return $this->repository->getLocationService()->loadLocation($bookmark->locationId); - }, $bookmarks); + $filter = new Filter(); + try { + $filter + ->withCriterion(new Criterion\Bookmark($currentUserId)) + ->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC)) + ->sliceBy($limit, $offset); + + $result = $this->repository->getlocationService()->find($filter, []); + } catch (\eZ\Publish\API\Repository\Exceptions\BadStateException $e) { + return new BookmarkList(); } + $list = new BookmarkList(); + $list->totalCount = $result->totalCount; + $list->items = $result->locations; + return $list; } diff --git a/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php b/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php index 5c23d98b3e..c21f197492 100644 --- a/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php +++ b/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php @@ -12,6 +12,7 @@ use eZ\Publish\API\Repository\LocationService; use eZ\Publish\API\Repository\PermissionResolver; use eZ\Publish\API\Repository\Values\Content\ContentInfo; +use eZ\Publish\API\Repository\Values\Content\LocationList; use eZ\Publish\Core\Repository\BookmarkService; use eZ\Publish\Core\Repository\Tests\Service\Mock\Base as BaseServiceMockTest; use eZ\Publish\Core\Repository\Values\Content\Location; @@ -219,28 +220,13 @@ public function testLoadBookmarks() $expectedItems = array_map(function ($locationId) { return $this->createLocation($locationId); }, range(1, $expectedTotalCount)); - - $this->bookmarkHandler - ->expects($this->once()) - ->method('countUserBookmarks') - ->with(self::CURRENT_USER_ID) - ->willReturn($expectedTotalCount); - - $this->bookmarkHandler - ->expects($this->once()) - ->method('loadUserBookmarks') - ->with(self::CURRENT_USER_ID, $offset, $limit) - ->willReturn(array_map(static function ($locationId) { - return new Bookmark(['locationId' => $locationId]); - }, range(1, $expectedTotalCount))); + $locationList = new LocationList(['totalCount' => $expectedTotalCount, 'locations' => $expectedItems]); $locationServiceMock = $this->createMock(LocationService::class); $locationServiceMock - ->expects($this->exactly($expectedTotalCount)) - ->method('loadLocation') - ->willReturnCallback(function ($locationId) { - return $this->createLocation($locationId); - }); + ->expects($this->once()) + ->method('find') + ->willReturn($locationList); $repository = $this->getRepositoryMock(); $repository @@ -254,27 +240,6 @@ public function testLoadBookmarks() $this->assertEquals($expectedItems, $bookmarks->items); } - /** - * @covers \eZ\Publish\Core\Repository\BookmarkService::loadBookmarks - */ - public function testLoadBookmarksEmptyList() - { - $this->bookmarkHandler - ->expects($this->once()) - ->method('countUserBookmarks') - ->with(self::CURRENT_USER_ID) - ->willReturn(0); - - $this->bookmarkHandler - ->expects($this->never()) - ->method('loadUserBookmarks'); - - $bookmarks = $this->createBookmarkService()->loadBookmarks(0, 10); - - $this->assertEquals(0, $bookmarks->totalCount); - $this->assertEmpty($bookmarks->items); - } - /** * @covers \eZ\Publish\Core\Repository\BookmarkService::isBookmarked */ diff --git a/eZ/Publish/SPI/Persistence/Bookmark/Handler.php b/eZ/Publish/SPI/Persistence/Bookmark/Handler.php index c335f40297..8a515d187c 100644 --- a/eZ/Publish/SPI/Persistence/Bookmark/Handler.php +++ b/eZ/Publish/SPI/Persistence/Bookmark/Handler.php @@ -41,6 +41,8 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra /** * Loads bookmarks owned by user. * + * @deprecated Please use LocationService::find() and Criterion\Bookmark instead. + * * @param int $userId * @param int $offset the start offset for paging * @param int $limit the number of bookmarked locations returned @@ -52,6 +54,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) /** * Count bookmarks owned by user. * + * @deprecated Please use LocationService::count() and Criterion\Bookmark instead. + * * @param int $userId * * @return int From 023111bb3534668e50b52fd5fbf7d82f00ec765d Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Fri, 7 Jun 2024 06:58:26 +0200 Subject: [PATCH 02/13] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception --- .../Query/Criterion/{Bookmark.php => IsBookmarked.php} | 4 ++-- .../Values/Content/Query/SortClause/BookmarkId.php | 2 +- eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php | 4 ++-- eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php | 4 ++-- .../Legacy/Bookmark/Gateway/DoctrineDatabase.php | 4 ++-- .../Legacy/Bookmark/Gateway/ExceptionConversion.php | 4 ++-- eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php | 4 ++-- .../CriterionQueryBuilder/Location/BookmarkQueryBuilder.php | 4 ++-- .../Location/BookmarkQueryBuilderTest.php | 6 +++--- eZ/Publish/Core/Repository/BookmarkService.php | 2 +- eZ/Publish/SPI/Persistence/Bookmark/Handler.php | 4 ++-- 11 files changed, 21 insertions(+), 21 deletions(-) rename eZ/Publish/API/Repository/Values/Content/Query/Criterion/{Bookmark.php => IsBookmarked.php} (91%) diff --git a/eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php b/eZ/Publish/API/Repository/Values/Content/Query/Criterion/IsBookmarked.php similarity index 91% rename from eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php rename to eZ/Publish/API/Repository/Values/Content/Query/Criterion/IsBookmarked.php index faf57568af..c713103b63 100644 --- a/eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php +++ b/eZ/Publish/API/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -19,10 +19,10 @@ * Supported operators: * - EQ: matches against a unique user id */ -class Bookmark extends Criterion implements FilteringCriterion +class IsBookmarked extends Criterion implements FilteringCriterion { /** - * Creates a new Bookmark criterion. + * Creates a new IsBookmarked criterion. * * @param int $value UserID for which bookmarked locations must be matched against * diff --git a/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php b/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php index 7b32cfb456..c18f71db8b 100644 --- a/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php +++ b/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php @@ -13,7 +13,7 @@ use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause; /** - * Sets sort direction on the bookmark id for a location query containing a Bookmark criterion. + * Sets sort direction on the Bookmark ID for a location query containing a IsBookmarked criterion. */ class BookmarkId extends SortClause implements FilteringSortClause { diff --git a/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php b/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php index 11a96d3081..1661edab9b 100644 --- a/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php +++ b/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php @@ -85,7 +85,7 @@ function (Bookmark $bookmark) { } /** - * @deprecated Please use LocationService::find() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. * * {@inheritdoc} */ @@ -101,7 +101,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. * * {@inheritdoc} */ diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php index 571a94f1aa..b11ba470a0 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php @@ -44,7 +44,7 @@ abstract public function loadBookmarkDataByUserIdAndLocationId(int $userId, arra /** * Load data for all bookmarks owned by given $userId. * - * @deprecated Please use LocationService::find() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. * * @param int $userId ID of user * @param int $offset Offset to start listing from, 0 by default @@ -57,7 +57,7 @@ abstract public function loadUserBookmarks(int $userId, int $offset = 0, int $li /** * Count bookmarks owned by given $userId. * - * @deprecated Please use LocationService::count() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. * * @param int $userId ID of user * diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php index b3ad0f6bf9..bbb8f6d056 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php @@ -101,7 +101,7 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati } /** - * @deprecated Please use LocationService::find() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. * * {@inheritdoc} */ @@ -125,7 +125,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. * * {@inheritdoc} */ diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php index d3b6e2c9ef..6fb769ae3c 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php @@ -57,7 +57,7 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati } /** - * @deprecated Please use LocationService::find() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. * * @param int $userId * @param int $offset @@ -75,7 +75,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. * * @param int $userId * diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php index fad593c7f8..afeb52a133 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php @@ -74,7 +74,7 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra } /** - * @deprecated Please use LocationService::find() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. * * {@inheritdoc} */ @@ -86,7 +86,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. * * {@inheritdoc} */ diff --git a/eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index 63150df80c..ad31c82a1f 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -9,7 +9,7 @@ namespace eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location; use Doctrine\DBAL\ParameterType; -use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Bookmark; +use eZ\Publish\API\Repository\Values\Content\Query\Criterion\IsBookmarked; use eZ\Publish\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase; use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder; use eZ\Publish\SPI\Repository\Values\Filter\FilteringCriterion; @@ -21,7 +21,7 @@ final class BookmarkQueryBuilder extends BaseLocationCriterionQueryBuilder { public function accepts(FilteringCriterion $criterion): bool { - return $criterion instanceof Bookmark; + return $criterion instanceof IsBookmarked; } public function buildQueryConstraint( diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php index eb3e52bae0..b70a0f9464 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -21,7 +21,7 @@ final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTes public function getFilteringCriteriaQueryData(): iterable { yield 'Bookmarks locations for user_id=14' => [ - new Criterion\Bookmark(14), + new Criterion\IsBookmarked(14), 'bookmark.user_id = :dcValue1', ['dcValue1' => 14], ]; @@ -29,8 +29,8 @@ public function getFilteringCriteriaQueryData(): iterable yield 'Bookmarks locations for user_id=14 OR user_id=7' => [ new Criterion\LogicalOr( [ - new Criterion\Bookmark(14), - new Criterion\Bookmark(7), + new Criterion\IsBookmarked(14), + new Criterion\IsBookmarked(7), ] ), '(bookmark.user_id = :dcValue1) OR (bookmark.user_id = :dcValue2)', diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index bf9914289f..455ae32bd3 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -103,7 +103,7 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList $filter = new Filter(); try { $filter - ->withCriterion(new Criterion\Bookmark($currentUserId)) + ->withCriterion(new Criterion\IsBookmarked($currentUserId)) ->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC)) ->sliceBy($limit, $offset); diff --git a/eZ/Publish/SPI/Persistence/Bookmark/Handler.php b/eZ/Publish/SPI/Persistence/Bookmark/Handler.php index 8a515d187c..2b111c412c 100644 --- a/eZ/Publish/SPI/Persistence/Bookmark/Handler.php +++ b/eZ/Publish/SPI/Persistence/Bookmark/Handler.php @@ -41,7 +41,7 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra /** * Loads bookmarks owned by user. * - * @deprecated Please use LocationService::find() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. * * @param int $userId * @param int $offset the start offset for paging @@ -54,7 +54,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) /** * Count bookmarks owned by user. * - * @deprecated Please use LocationService::count() and Criterion\Bookmark instead. + * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. * * @param int $userId * From 8c2de22fc5df0fc03cf6f7f24a2b40d43a35d34b Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 25 Jun 2024 13:44:26 +0200 Subject: [PATCH 03/13] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski --- .../Values/Content/Query/SortClause/BookmarkId.php | 5 ----- .../Legacy/Bookmark/Gateway/ExceptionConversion.php | 8 -------- 2 files changed, 13 deletions(-) diff --git a/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php b/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php index c18f71db8b..ea128295d6 100644 --- a/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php +++ b/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php @@ -17,11 +17,6 @@ */ class BookmarkId extends SortClause implements FilteringSortClause { - /** - * Constructs a new BookmarkId SortClause. - * - * @param string $sortDirection - */ public function __construct(string $sortDirection = Query::SORT_ASC) { parent::__construct('id', $sortDirection); diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php index 6fb769ae3c..072de51cad 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php @@ -59,10 +59,6 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati /** * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. * - * @param int $userId - * @param int $offset - * @param int $limit - * * @return array */ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array @@ -76,10 +72,6 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) /** * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. - * - * @param int $userId - * - * @return int */ public function countUserBookmarks(int $userId): int { From 6091d1d5caa140f7435307d5f29f1f33549421fe Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 25 Jun 2024 13:49:56 +0200 Subject: [PATCH 04/13] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - removed covers --- .../Location/BookmarkQueryBuilderTest.php | 4 ---- .../Core/Repository/Tests/Service/Mock/BookmarkTest.php | 3 --- 2 files changed, 7 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php index b70a0f9464..88e1127286 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php +++ b/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -12,10 +12,6 @@ use eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder; use eZ\Publish\Core\Persistence\Legacy\Tests\Filter\BaseCriterionVisitorQueryBuilderTestCase; -/** - * @covers \eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder::buildQueryConstraint - * @covers \eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder::accepts - */ final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTestCase { public function getFilteringCriteriaQueryData(): iterable diff --git a/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php b/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php index c21f197492..4150f4351f 100644 --- a/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php +++ b/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php @@ -254,9 +254,6 @@ public function testLocationShouldNotBeBookmarked() $this->assertFalse($this->createBookmarkService()->isBookmarked($this->createLocation(self::LOCATION_ID))); } - /** - * @covers \eZ\Publish\Core\Repository\BookmarkService::isBookmarked - */ public function testLocationShouldBeBookmarked() { $this->bookmarkHandler From fabbe470d2ac6bb47c8b009dc3c2a236f04b654a Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 25 Jun 2024 14:39:45 +0200 Subject: [PATCH 05/13] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - Changed to ibexa namespace for new classes --- .../Values/Content/Query/Criterion/IsBookmarked.php | 2 +- .../Values/Content/Query/SortClause/BookmarkId.php | 2 +- .../Location/BookmarkQueryBuilder.php | 5 +++-- .../Location/BookmarkQueryBuilderTest.php | 9 +++++---- .../Bookmark/IdSortClauseQueryBuilder.php | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) rename {eZ/Publish/API => src/contracts}/Repository/Values/Content/Query/Criterion/IsBookmarked.php (94%) rename {eZ/Publish/API => src/contracts}/Repository/Values/Content/Query/SortClause/BookmarkId.php (90%) rename {eZ/Publish/Core => src/lib}/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php (82%) rename {eZ/Publish/Core/Persistence/Legacy/Tests => tests/lib/Persistence/Legacy}/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php (75%) rename {eZ/Publish/Core => vendor/ibexa/core/src/lib}/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php (92%) diff --git a/eZ/Publish/API/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php similarity index 94% rename from eZ/Publish/API/Repository/Values/Content/Query/Criterion/IsBookmarked.php rename to src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index c713103b63..24a56f9c20 100644 --- a/eZ/Publish/API/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -6,7 +6,7 @@ */ declare(strict_types=1); -namespace eZ\Publish\API\Repository\Values\Content\Query\Criterion; +namespace Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use eZ\Publish\API\Repository\Values\Content\Query\Criterion; use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Operator\Specifications; diff --git a/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php b/src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php similarity index 90% rename from eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php rename to src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php index ea128295d6..3574b68695 100644 --- a/eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php +++ b/src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php @@ -6,7 +6,7 @@ */ declare(strict_types=1); -namespace eZ\Publish\API\Repository\Values\Content\Query\SortClause; +namespace Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause; use eZ\Publish\API\Repository\Values\Content\Query; use eZ\Publish\API\Repository\Values\Content\Query\SortClause; diff --git a/eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php similarity index 82% rename from eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php rename to src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index ad31c82a1f..a6e71b376f 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -6,13 +6,14 @@ */ declare(strict_types=1); -namespace eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location; +namespace Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location; use Doctrine\DBAL\ParameterType; -use eZ\Publish\API\Repository\Values\Content\Query\Criterion\IsBookmarked; use eZ\Publish\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase; +use eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BaseLocationCriterionQueryBuilder; use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder; use eZ\Publish\SPI\Repository\Values\Filter\FilteringCriterion; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked; /** * @internal for internal use by Repository Filtering diff --git a/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php similarity index 75% rename from eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php rename to tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php index 88e1127286..7c615fc563 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php +++ b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -6,11 +6,12 @@ */ declare(strict_types=1); -namespace eZ\Publish\Core\Persistence\Legacy\Tests\Filter\CriterionQueryBuilder\Location; +namespace Ibexa\Tests\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location; -use eZ\Publish\API\Repository\Values\Content\Query\Criterion; -use eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder; +use eZ\Publish\API\Repository\Values\Content\Query\Criterion as EzCriterion; use eZ\Publish\Core\Persistence\Legacy\Tests\Filter\BaseCriterionVisitorQueryBuilderTestCase; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; +use Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder; final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTestCase { @@ -23,7 +24,7 @@ public function getFilteringCriteriaQueryData(): iterable ]; yield 'Bookmarks locations for user_id=14 OR user_id=7' => [ - new Criterion\LogicalOr( + new EzCriterion\LogicalOr( [ new Criterion\IsBookmarked(14), new Criterion\IsBookmarked(7), diff --git a/eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php b/vendor/ibexa/core/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php similarity index 92% rename from eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php rename to vendor/ibexa/core/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php index ac2c8ea4e0..9e8f4adced 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php +++ b/vendor/ibexa/core/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php @@ -6,7 +6,7 @@ */ declare(strict_types=1); -namespace eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark; +namespace Ibexa\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark; use eZ\Publish\API\Repository\Values\Content\Query\SortClause\BookmarkId; use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder; From 7aa26516555aebb54d799f037df382d2c7c62d5a Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 26 Jun 2024 13:36:48 +0200 Subject: [PATCH 06/13] fixup! fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - Changed to ibexa namespace for new classes --- eZ/Publish/Core/Repository/BookmarkService.php | 5 +++-- .../legacy/filter/query_builders.yaml | 5 +++++ .../Values/Content/Query/Criterion/IsBookmarked.php | 1 + .../Bookmark/IdSortClauseQueryBuilder.php | 2 +- .../Location/BookmarkQueryBuilderTest.php | 12 ++++++------ 5 files changed, 16 insertions(+), 9 deletions(-) rename {vendor/ibexa/core/src => src}/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php (92%) diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index 455ae32bd3..845f88768a 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -14,12 +14,13 @@ use eZ\Publish\API\Repository\Values\Bookmark\BookmarkList; use eZ\Publish\API\Repository\Values\Content\Location; use eZ\Publish\API\Repository\Values\Content\Query; -use eZ\Publish\API\Repository\Values\Content\Query\Criterion; -use eZ\Publish\API\Repository\Values\Content\Query\SortClause; use eZ\Publish\API\Repository\Values\Filter\Filter; use eZ\Publish\Core\Base\Exceptions\InvalidArgumentException; use eZ\Publish\SPI\Persistence\Bookmark\CreateStruct; use eZ\Publish\SPI\Persistence\Bookmark\Handler as BookmarkHandler; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause; + class BookmarkService implements BookmarkServiceInterface { diff --git a/eZ/Publish/Core/settings/storage_engines/legacy/filter/query_builders.yaml b/eZ/Publish/Core/settings/storage_engines/legacy/filter/query_builders.yaml index 663da66fbf..231658d505 100644 --- a/eZ/Publish/Core/settings/storage_engines/legacy/filter/query_builders.yaml +++ b/eZ/Publish/Core/settings/storage_engines/legacy/filter/query_builders.yaml @@ -9,6 +9,11 @@ services: eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\: resource: '../../../../Persistence/Legacy/Filter/CriterionQueryBuilder/*' + Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\: + resource: '../../../../../../../src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/*' + eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\: resource: '../../../../Persistence/Legacy/Filter/SortClauseQueryBuilder/*' + Ibexa\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\: + resource: '../../../../../../../src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/*' diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index 24a56f9c20..bfea9c0cda 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -9,6 +9,7 @@ namespace Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use eZ\Publish\API\Repository\Values\Content\Query\Criterion; +use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Operator; use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Operator\Specifications; use eZ\Publish\SPI\Repository\Values\Filter\FilteringCriterion; diff --git a/vendor/ibexa/core/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php similarity index 92% rename from vendor/ibexa/core/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php rename to src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php index 9e8f4adced..afd9d7bcb1 100644 --- a/vendor/ibexa/core/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php @@ -8,7 +8,7 @@ namespace Ibexa\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark; -use eZ\Publish\API\Repository\Values\Content\Query\SortClause\BookmarkId; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\BookmarkId; use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder; use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause; use eZ\Publish\SPI\Repository\Values\Filter\SortClauseQueryBuilder; diff --git a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php index 7c615fc563..6e45c6b7a4 100644 --- a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php +++ b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -8,9 +8,9 @@ namespace Ibexa\Tests\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location; -use eZ\Publish\API\Repository\Values\Content\Query\Criterion as EzCriterion; +use eZ\Publish\API\Repository\Values\Content\Query\Criterion; use eZ\Publish\Core\Persistence\Legacy\Tests\Filter\BaseCriterionVisitorQueryBuilderTestCase; -use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion as IbexaCriterion; use Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder; final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTestCase @@ -18,16 +18,16 @@ final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTes public function getFilteringCriteriaQueryData(): iterable { yield 'Bookmarks locations for user_id=14' => [ - new Criterion\IsBookmarked(14), + new IbexaCriterion\IsBookmarked(14), 'bookmark.user_id = :dcValue1', ['dcValue1' => 14], ]; yield 'Bookmarks locations for user_id=14 OR user_id=7' => [ - new EzCriterion\LogicalOr( + new Criterion\LogicalOr( [ - new Criterion\IsBookmarked(14), - new Criterion\IsBookmarked(7), + new IbexaCriterion\IsBookmarked(14), + new IbexaCriterion\IsBookmarked(7), ] ), '(bookmark.user_id = :dcValue1) OR (bookmark.user_id = :dcValue2)', From 8483861fa87eaf7929084818623dace97d088349 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 26 Jun 2024 13:48:49 +0200 Subject: [PATCH 07/13] fixup! fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - Changed to ibexa namespace for new classes --- eZ/Publish/Core/Repository/BookmarkService.php | 1 - .../Bookmark/IdSortClauseQueryBuilder.php | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index 845f88768a..25578367fc 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -21,7 +21,6 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause; - class BookmarkService implements BookmarkServiceInterface { /** @var \eZ\Publish\API\Repository\Repository */ diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php index afd9d7bcb1..0e5613b91b 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php @@ -8,10 +8,10 @@ namespace Ibexa\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark; -use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\BookmarkId; use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder; use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause; use eZ\Publish\SPI\Repository\Values\Filter\SortClauseQueryBuilder; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\BookmarkId; class IdSortClauseQueryBuilder implements SortClauseQueryBuilder { From 9c9b56e58a2405e152ce0f5a1145f0af0d67b94c Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 26 Jun 2024 15:15:23 +0200 Subject: [PATCH 08/13] fixup! fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - changed deprecations messages --- eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php | 4 ++-- eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php | 4 ++-- .../Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php | 4 ++-- .../Legacy/Bookmark/Gateway/ExceptionConversion.php | 4 ++-- eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php | 4 ++-- eZ/Publish/SPI/Persistence/Bookmark/Handler.php | 4 ++-- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php b/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php index 1661edab9b..9c901b6f54 100644 --- a/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php +++ b/eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php @@ -85,7 +85,7 @@ function (Bookmark $bookmark) { } /** - * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. + * @deprecated The "BookmarkHandler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. * * {@inheritdoc} */ @@ -101,7 +101,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. + * @deprecated The "BookmarkHandler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. * * {@inheritdoc} */ diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php index b11ba470a0..d7355417f1 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php @@ -44,7 +44,7 @@ abstract public function loadBookmarkDataByUserIdAndLocationId(int $userId, arra /** * Load data for all bookmarks owned by given $userId. * - * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. + * @deprecated Gateway::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. * * @param int $userId ID of user * @param int $offset Offset to start listing from, 0 by default @@ -57,7 +57,7 @@ abstract public function loadUserBookmarks(int $userId, int $offset = 0, int $li /** * Count bookmarks owned by given $userId. * - * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. + * @deprecated The "Gateway::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. * * @param int $userId ID of user * diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php index bbb8f6d056..25b98677a3 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php @@ -101,7 +101,7 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati } /** - * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. + * @deprecated The "DoctrineDatabase::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. * * {@inheritdoc} */ @@ -125,7 +125,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. + * @deprecated The "DoctrineDatabase::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. * * {@inheritdoc} */ diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php index 072de51cad..b9f299fd56 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php @@ -57,7 +57,7 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati } /** - * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. + * @deprecated The "ExceptionConversion::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. * * @return array */ @@ -71,7 +71,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. + * @deprecated The "ExceptionConversion::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. */ public function countUserBookmarks(int $userId): int { diff --git a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php index afeb52a133..c40d7a2217 100644 --- a/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php +++ b/eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php @@ -74,7 +74,7 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra } /** - * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. + * @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. * * {@inheritdoc} */ @@ -86,7 +86,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. + * @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. * * {@inheritdoc} */ diff --git a/eZ/Publish/SPI/Persistence/Bookmark/Handler.php b/eZ/Publish/SPI/Persistence/Bookmark/Handler.php index 2b111c412c..9b69dae436 100644 --- a/eZ/Publish/SPI/Persistence/Bookmark/Handler.php +++ b/eZ/Publish/SPI/Persistence/Bookmark/Handler.php @@ -41,7 +41,7 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra /** * Loads bookmarks owned by user. * - * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead. + * @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. * * @param int $userId * @param int $offset the start offset for paging @@ -54,7 +54,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) /** * Count bookmarks owned by user. * - * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead. + * @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. * * @param int $userId * From f620af5d018fa1a1d2241ea6aabca19305c9cd32 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 27 Jun 2024 13:04:46 +0200 Subject: [PATCH 09/13] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception --- .../API/Repository/Tests/LocationServiceTest.php | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php index 4238ae4671..f92c9ab5b1 100644 --- a/eZ/Publish/API/Repository/Tests/LocationServiceTest.php +++ b/eZ/Publish/API/Repository/Tests/LocationServiceTest.php @@ -1987,19 +1987,8 @@ public function testBookmarksAreSwappedAfterSwapLocation() $afterSwap = $bookmarkService->loadBookmarks(); /* END: Use Case */ - $expectedIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) { - if ($item->id === $demoDesignLocationId) { - return $contactUsLocationId; - } - - return $item->id; - }, $beforeSwap->items); - - $actualIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) { - return $item->id; - }, $afterSwap->items); - - $this->assertEquals($expectedIdsAfter, $actualIdsAfter); + $this->assertEquals($contactUsLocationId, $afterSwap->items[0]->id); + $this->assertEquals($beforeSwap->items[1]->id, $afterSwap->items[1]->id); } /** From 26655788cb72177b51fc7728095a4a02f2810f44 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 27 Jun 2024 13:08:00 +0200 Subject: [PATCH 10/13] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception --- eZ/Publish/Core/Repository/BookmarkService.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index 25578367fc..6a6cc9f926 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -10,6 +10,7 @@ use Exception; use eZ\Publish\API\Repository\BookmarkService as BookmarkServiceInterface; +use eZ\Publish\API\Repository\Exceptions\BadStateException; use eZ\Publish\API\Repository\Repository as RepositoryInterface; use eZ\Publish\API\Repository\Values\Bookmark\BookmarkList; use eZ\Publish\API\Repository\Values\Content\Location; @@ -108,7 +109,7 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList ->sliceBy($limit, $offset); $result = $this->repository->getlocationService()->find($filter, []); - } catch (\eZ\Publish\API\Repository\Exceptions\BadStateException $e) { + } catch (BadStateException $e) { return new BookmarkList(); } From f208d3d584cc4b351b3701932b2cedd2e0e9a3da Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 3 Oct 2024 13:37:29 +0200 Subject: [PATCH 11/13] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - Added logger in BookmarkService --- eZ/Publish/Core/Repository/BookmarkService.php | 12 ++++++++---- eZ/Publish/Core/Repository/Repository.php | 3 ++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index 6a6cc9f926..6b52fe684d 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -21,6 +21,8 @@ use eZ\Publish\SPI\Persistence\Bookmark\Handler as BookmarkHandler; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; class BookmarkService implements BookmarkServiceInterface { @@ -30,16 +32,16 @@ class BookmarkService implements BookmarkServiceInterface /** @var \eZ\Publish\SPI\Persistence\Bookmark\Handler */ protected $bookmarkHandler; + private LoggerInterface $logger; + /** * BookmarkService constructor. - * - * @param \eZ\Publish\API\Repository\Repository $repository - * @param \eZ\Publish\SPI\Persistence\Bookmark\Handler $bookmarkHandler */ - public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler) + public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler, LoggerInterface $logger = null) { $this->repository = $repository; $this->bookmarkHandler = $bookmarkHandler; + $this->logger = $logger ?? new NullLogger(); } /** @@ -110,6 +112,8 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList $result = $this->repository->getlocationService()->find($filter, []); } catch (BadStateException $e) { + $this->logger->debug($e); + return new BookmarkList(); } diff --git a/eZ/Publish/Core/Repository/Repository.php b/eZ/Publish/Core/Repository/Repository.php index a1459a7a8f..028bb5ca20 100644 --- a/eZ/Publish/Core/Repository/Repository.php +++ b/eZ/Publish/Core/Repository/Repository.php @@ -599,7 +599,8 @@ public function getBookmarkService(): BookmarkServiceInterface if ($this->bookmarkService === null) { $this->bookmarkService = new BookmarkService( $this, - $this->persistenceHandler->bookmarkHandler() + $this->persistenceHandler->bookmarkHandler(), + $this->logger ); } From ffa945c4f624e377904233f7f05821064d6f607d Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 3 Oct 2024 13:56:48 +0200 Subject: [PATCH 12/13] fixup! fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - Added logger in BookmarkService --- eZ/Publish/Core/Repository/BookmarkService.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index 6b52fe684d..1d13afc8d1 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -32,7 +32,8 @@ class BookmarkService implements BookmarkServiceInterface /** @var \eZ\Publish\SPI\Persistence\Bookmark\Handler */ protected $bookmarkHandler; - private LoggerInterface $logger; + /** @var \Psr\Log\LoggerInterface */ + private $logger; /** * BookmarkService constructor. From 203853c4d65b73267e9559579b8bc49135f05c77 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Fri, 8 Nov 2024 10:24:44 +0100 Subject: [PATCH 13/13] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Paweł Niedzielski Co-authored-by: Konrad Oboza --- eZ/Publish/Core/Repository/BookmarkService.php | 4 +++- .../Core/Repository/Tests/Service/Mock/BookmarkTest.php | 2 +- .../Values/Content/Query/Criterion/IsBookmarked.php | 5 ++--- .../Values/Content/Query/SortClause/BookmarkId.php | 4 ++-- .../Bookmark/IdSortClauseQueryBuilder.php | 2 +- 5 files changed, 9 insertions(+), 8 deletions(-) diff --git a/eZ/Publish/Core/Repository/BookmarkService.php b/eZ/Publish/Core/Repository/BookmarkService.php index 1d13afc8d1..2b97b20496 100644 --- a/eZ/Publish/Core/Repository/BookmarkService.php +++ b/eZ/Publish/Core/Repository/BookmarkService.php @@ -113,7 +113,9 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList $result = $this->repository->getlocationService()->find($filter, []); } catch (BadStateException $e) { - $this->logger->debug($e); + $this->logger->debug($e, [ + 'exception' => $e, + ]); return new BookmarkList(); } diff --git a/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php b/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php index 4150f4351f..8c23c0dbfc 100644 --- a/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php +++ b/eZ/Publish/Core/Repository/Tests/Service/Mock/BookmarkTest.php @@ -224,7 +224,7 @@ public function testLoadBookmarks() $locationServiceMock = $this->createMock(LocationService::class); $locationServiceMock - ->expects($this->once()) + ->expects(self::once()) ->method('find') ->willReturn($locationList); diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index bfea9c0cda..15be989a8d 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -16,16 +16,15 @@ /** * A criterion that matches locations of bookmarks for a given user id. * - * * Supported operators: * - EQ: matches against a unique user id */ -class IsBookmarked extends Criterion implements FilteringCriterion +final class IsBookmarked extends Criterion implements FilteringCriterion { /** * Creates a new IsBookmarked criterion. * - * @param int $value UserID for which bookmarked locations must be matched against + * @param int $value user id for which bookmarked locations must be matched against * * @throws \InvalidArgumentException if a non numeric id is given * @throws \InvalidArgumentException if the value type doesn't match the operator diff --git a/src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php b/src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php index 3574b68695..b00d4488d4 100644 --- a/src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php +++ b/src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php @@ -13,9 +13,9 @@ use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause; /** - * Sets sort direction on the Bookmark ID for a location query containing a IsBookmarked criterion. + * Sets sort direction on the bookmark id for a location query containing IsBookmarked criterion. */ -class BookmarkId extends SortClause implements FilteringSortClause +final class BookmarkId extends SortClause implements FilteringSortClause { public function __construct(string $sortDirection = Query::SORT_ASC) { diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php index 0e5613b91b..e9859519b8 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php @@ -13,7 +13,7 @@ use eZ\Publish\SPI\Repository\Values\Filter\SortClauseQueryBuilder; use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\BookmarkId; -class IdSortClauseQueryBuilder implements SortClauseQueryBuilder +final class IdSortClauseQueryBuilder implements SortClauseQueryBuilder { public function accepts(FilteringSortClause $sortClause): bool {