From 54a8758ef3c4a3a230f44ab8baf5a3ed6fb2f961 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Fri, 7 Feb 2025 15:06:45 +0100 Subject: [PATCH 01/29] IBX-6773: Bookmarks for non-accessible contents cause exception --- phpstan-baseline.neon | 23 ++++++--- .../Persistence/Bookmark/Handler.php | 4 ++ .../Content/Query/Criterion/IsBookmarked.php | 42 ++++++++++++++++ .../Content/Query/SortClause/BookmarkId.php | 24 ++++++++++ src/lib/Persistence/Cache/BookmarkHandler.php | 4 ++ .../Persistence/Legacy/Bookmark/Gateway.php | 4 ++ .../Bookmark/Gateway/DoctrineDatabase.php | 4 ++ .../Bookmark/Gateway/ExceptionConversion.php | 8 ++++ .../Persistence/Legacy/Bookmark/Handler.php | 4 ++ .../Location/BookmarkQueryBuilder.php | 48 +++++++++++++++++++ .../Bookmark/IdSortClauseQueryBuilder.php | 31 ++++++++++++ src/lib/Repository/BookmarkService.php | 40 ++++++++++++---- src/lib/Repository/Repository.php | 3 +- .../Core/Repository/LocationServiceTest.php | 8 ++-- .../Location/BookmarkQueryBuilderTest.php | 47 ++++++++++++++++++ .../Repository/Service/Mock/BookmarkTest.php | 48 ++----------------- 16 files changed, 279 insertions(+), 63 deletions(-) create mode 100644 src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php create mode 100644 src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php create mode 100644 src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php create mode 100644 src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php create mode 100644 tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 42a35b167e..4b161d74da 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -21888,6 +21888,17 @@ parameters: count: 1 path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/DepthQueryBuilder.php + - + message: "#^Cannot access offset 0 on array\\\\|bool\\|float\\|int\\|string\\.$#" + count: 1 + path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php + + - + message: '#^Method Ibexa\\Core\\Persistence\\Legacy\\Filter\\CriterionQueryBuilder\\Location\\BookmarkQueryBuilder\:\:buildQueryConstraint\(\) never returns null so it can be removed from the return type\.$#' + identifier: return.unusedType + count: 1 + path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php + - message: '#^Parameter \#2 \$criterionValue of method Ibexa\\Contracts\\Core\\Persistence\\Filter\\Doctrine\\FilteringQueryBuilder\:\:buildOperatorBasedCriterionConstraint\(\) expects array, array\\|bool\|float\|int\|string given\.$#' identifier: argument.type @@ -22308,6 +22319,12 @@ parameters: count: 1 path: src/lib/Persistence/Legacy/Filter/Handler/LocationFilteringHandler.php + - + message: '#^PHPDoc tag @var with type Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Query\\SortClause is not subtype of native type Ibexa\\Contracts\\Core\\Repository\\Values\\Filter\\FilteringSortClause\.$#' + identifier: varTag.nativeType + count: 1 + path: src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php + - message: '#^PHPDoc tag @var with type Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Query\\SortClause is not subtype of native type Ibexa\\Contracts\\Core\\Repository\\Values\\Filter\\FilteringSortClause\.$#' identifier: varTag.nativeType @@ -69570,12 +69587,6 @@ parameters: count: 1 path: tests/lib/Repository/Service/Mock/BookmarkTest.php - - - message: '#^Method Ibexa\\Tests\\Core\\Repository\\Service\\Mock\\BookmarkTest\:\:testLoadBookmarksEmptyList\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/lib/Repository/Service/Mock/BookmarkTest.php - - message: '#^Method Ibexa\\Tests\\Core\\Repository\\Service\\Mock\\BookmarkTest\:\:testLocationShouldBeBookmarked\(\) has no return type specified\.$#' identifier: missingType.return diff --git a/src/contracts/Persistence/Bookmark/Handler.php b/src/contracts/Persistence/Bookmark/Handler.php index f9ef1a1cae..714d85a736 100644 --- a/src/contracts/Persistence/Bookmark/Handler.php +++ b/src/contracts/Persistence/Bookmark/Handler.php @@ -50,6 +50,8 @@ public function loadUserIdsByLocation(Location $location): array; /** * Loads bookmarks owned by user. * + * @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. + * * @param int $userId * @param int $offset the start offset for paging * @param int $limit the number of bookmarked locations returned @@ -61,6 +63,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) /** * Count bookmarks owned by user. * + * @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. + * * @param int $userId * * @return int diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php new file mode 100644 index 0000000000..5ce8b9709c --- /dev/null +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -0,0 +1,42 @@ +joinOnce( + 'location', + DoctrineDatabase::TABLE_BOOKMARKS, + 'bookmark', + 'location.node_id = bookmark.node_id' + ); + + /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */ + return $queryBuilder->expr()->eq( + 'bookmark.user_id', + $queryBuilder->createNamedParameter( + (int)$criterion->value[0], + ParameterType::INTEGER + ) + ); + } +} diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php new file mode 100644 index 0000000000..50e85f0858 --- /dev/null +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php @@ -0,0 +1,31 @@ +addSelect('bookmark.id'); + $queryBuilder->addOrderBy('bookmark.id', $sortClause->direction); + } +} diff --git a/src/lib/Repository/BookmarkService.php b/src/lib/Repository/BookmarkService.php index a74c79d5cb..1ee28c7989 100644 --- a/src/lib/Repository/BookmarkService.php +++ b/src/lib/Repository/BookmarkService.php @@ -9,14 +9,20 @@ namespace Ibexa\Core\Repository; use Exception; -use Ibexa\Contracts\Core\Persistence\Bookmark\Bookmark; use Ibexa\Contracts\Core\Persistence\Bookmark\CreateStruct; use Ibexa\Contracts\Core\Persistence\Bookmark\Handler as BookmarkHandler; use Ibexa\Contracts\Core\Repository\BookmarkService as BookmarkServiceInterface; +use Ibexa\Contracts\Core\Repository\Exceptions\BadStateException; use Ibexa\Contracts\Core\Repository\Repository as RepositoryInterface; use Ibexa\Contracts\Core\Repository\Values\Bookmark\BookmarkList; use Ibexa\Contracts\Core\Repository\Values\Content\Location; +use Ibexa\Contracts\Core\Repository\Values\Content\Query; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause; +use Ibexa\Contracts\Core\Repository\Values\Filter\Filter; use Ibexa\Core\Base\Exceptions\InvalidArgumentException; +use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; class BookmarkService implements BookmarkServiceInterface { @@ -26,16 +32,20 @@ class BookmarkService implements BookmarkServiceInterface /** @var \Ibexa\Contracts\Core\Persistence\Bookmark\Handler */ protected $bookmarkHandler; + /** @var \Psr\Log\LoggerInterface */ + private $logger; + /** * BookmarkService constructor. * * @param \Ibexa\Contracts\Core\Repository\Repository $repository * @param \Ibexa\Contracts\Core\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(); } /** @@ -97,16 +107,26 @@ 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\IsBookmarked($currentUserId)) + ->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC)) + ->sliceBy($limit, $offset); + + $result = $this->repository->getlocationService()->find($filter, []); + } catch (BadStateException $e) { + $this->logger->debug($e->getMessage(), [ + 'exception' => $e, + ]); + + return new BookmarkList(); } + $list = new BookmarkList(); + $list->totalCount = $result->totalCount; + $list->items = $result->locations; + return $list; } diff --git a/src/lib/Repository/Repository.php b/src/lib/Repository/Repository.php index fa165c2222..81fccd268d 100644 --- a/src/lib/Repository/Repository.php +++ b/src/lib/Repository/Repository.php @@ -608,7 +608,8 @@ public function getBookmarkService(): BookmarkServiceInterface if ($this->bookmarkService === null) { $this->bookmarkService = new BookmarkService( $this, - $this->persistenceHandler->bookmarkHandler() + $this->persistenceHandler->bookmarkHandler(), + $this->logger ); } diff --git a/tests/integration/Core/Repository/LocationServiceTest.php b/tests/integration/Core/Repository/LocationServiceTest.php index 350347bda4..dc38cdb5f3 100644 --- a/tests/integration/Core/Repository/LocationServiceTest.php +++ b/tests/integration/Core/Repository/LocationServiceTest.php @@ -1968,6 +1968,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(); @@ -1975,6 +1976,7 @@ public function testBookmarksAreSwappedAfterSwapLocation() $mediaLocation = $locationService->loadLocation($mediaLocationId); $demoDesignLocation = $locationService->loadLocation($demoDesignLocationId); + $contactUsLocation = $locationService->loadLocation($contactUsLocationId); // Bookmark locations $bookmarkService->createBookmark($mediaLocation); @@ -1983,13 +1985,13 @@ 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); + $this->assertEquals($contactUsLocationId, $afterSwap->items[0]->id); + $this->assertEquals($beforeSwap->items[1]->id, $afterSwap->items[1]->id); } /** diff --git a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php new file mode 100644 index 0000000000..b632ac35b2 --- /dev/null +++ b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -0,0 +1,47 @@ +}> + * + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidCriterionArgumentException + */ + public function getFilteringCriteriaQueryData(): iterable + { + yield 'Bookmarks locations for user_id=14' => [ + new Criterion\IsBookmarked(14), + 'bookmark.user_id = :dcValue1', + ['dcValue1' => 14], + ]; + + yield 'Bookmarks locations for user_id=14 OR user_id=7' => [ + new Criterion\LogicalOr( + [ + new Criterion\IsBookmarked(14), + new Criterion\IsBookmarked(7), + ] + ), + '(bookmark.user_id = :dcValue1) OR (bookmark.user_id = :dcValue2)', + ['dcValue1' => 14, 'dcValue2' => 7], + ]; + } + + protected function getCriterionQueryBuilders(): iterable + { + return [new BookmarkQueryBuilder()]; + } +} diff --git a/tests/lib/Repository/Service/Mock/BookmarkTest.php b/tests/lib/Repository/Service/Mock/BookmarkTest.php index 536a0c2390..876dc4a8e7 100644 --- a/tests/lib/Repository/Service/Mock/BookmarkTest.php +++ b/tests/lib/Repository/Service/Mock/BookmarkTest.php @@ -15,6 +15,7 @@ use Ibexa\Contracts\Core\Repository\LocationService; use Ibexa\Contracts\Core\Repository\PermissionResolver; use Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo; +use Ibexa\Contracts\Core\Repository\Values\Content\LocationList; use Ibexa\Core\Repository\BookmarkService; use Ibexa\Core\Repository\Values\Content\Location; use Ibexa\Core\Repository\Values\User\UserReference; @@ -220,28 +221,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(self::once()) + ->method('find') + ->willReturn($locationList); $repository = $this->getRepositoryMock(); $repository @@ -255,27 +241,6 @@ public function testLoadBookmarks() $this->assertEquals($expectedItems, $bookmarks->items); } - /** - * @covers \Ibexa\Contracts\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 \Ibexa\Contracts\Core\Repository\BookmarkService::isBookmarked */ @@ -290,9 +255,6 @@ public function testLocationShouldNotBeBookmarked() $this->assertFalse($this->createBookmarkService()->isBookmarked($this->createLocation(self::LOCATION_ID))); } - /** - * @covers \Ibexa\Contracts\Core\Repository\BookmarkService::isBookmarked - */ public function testLocationShouldBeBookmarked() { $this->bookmarkHandler From 5324ef223f7f58ff73e5a57355eaf2fecc073181 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 24 Feb 2026 08:44:42 +0100 Subject: [PATCH 02/29] Fixed PHPStan error for URLAliasService --- phpstan-baseline.neon | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4b161d74da..9cb2e68834 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -17706,12 +17706,6 @@ parameters: count: 1 path: src/lib/Persistence/Cache/UserPreferenceHandler.php - - - message: '#^Return type \(Doctrine\\ORM\\Mapping\\ClassMetadataFactory\) of method Ibexa\\Core\\Persistence\\Doctrine\\SiteAccessAwareEntityManager\:\:getMetadataFactory\(\) should be compatible with return type \(Doctrine\\Persistence\\Mapping\\ClassMetadataFactory\\>\) of method Doctrine\\Persistence\\ObjectManager\:\:getMetadataFactory\(\)$#' - identifier: method.childReturnType - count: 2 - path: src/lib/Persistence/Doctrine/SiteAccessAwareEntityManager.php - - message: '#^Property Ibexa\\Core\\Persistence\\FieldTypeRegistry\:\:\$coreFieldTypes \(array\\) does not accept array\\.$#' identifier: assign.propertyType @@ -25367,6 +25361,12 @@ parameters: count: 1 path: src/lib/Repository/TrashService.php + - + message: '#^Class Ibexa\\Contracts\\Core\\Persistence\\Content\\UrlAlias referenced with incorrect case\: Ibexa\\Contracts\\Core\\Persistence\\Content\\URLAlias\.$#' + identifier: class.nameCase + count: 2 + path: src/lib/Repository/URLAliasService.php + - message: '#^Method Ibexa\\Core\\Repository\\URLAliasService\:\:choosePrioritizedLanguageCode\(\) has parameter \$entries with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue From e8062af78630b3872e1bba33171631332f154920 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 24 Feb 2026 09:19:54 +0100 Subject: [PATCH 03/29] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - PHPStand and BookmarkQueryBuilder --- phpstan-baseline.neon | 11 ----------- .../Location/BookmarkQueryBuilder.php | 13 +++++++++++-- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 9cb2e68834..f15029435a 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -21882,17 +21882,6 @@ parameters: count: 1 path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/DepthQueryBuilder.php - - - message: "#^Cannot access offset 0 on array\\\\|bool\\|float\\|int\\|string\\.$#" - count: 1 - path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php - - - - message: '#^Method Ibexa\\Core\\Persistence\\Legacy\\Filter\\CriterionQueryBuilder\\Location\\BookmarkQueryBuilder\:\:buildQueryConstraint\(\) never returns null so it can be removed from the return type\.$#' - identifier: return.unusedType - count: 1 - path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php - - message: '#^Parameter \#2 \$criterionValue of method Ibexa\\Contracts\\Core\\Persistence\\Filter\\Doctrine\\FilteringQueryBuilder\:\:buildOperatorBasedCriterionConstraint\(\) expects array, array\\|bool\|float\|int\|string given\.$#' identifier: argument.type diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index 994f89a55b..bf8f8712b9 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -27,7 +27,7 @@ public function accepts(FilteringCriterion $criterion): bool public function buildQueryConstraint( FilteringQueryBuilder $queryBuilder, FilteringCriterion $criterion - ): ?string { + ): string { $queryBuilder ->joinOnce( 'location', @@ -37,10 +37,19 @@ public function buildQueryConstraint( ); /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */ + $value = $criterion->value; + + if (\is_array($value)) { + if (!isset($value[0])) { + throw new \InvalidArgumentException('IsBookmarked criterion value must contain userId at index 0.'); + } + $value = $value[0]; + } + return $queryBuilder->expr()->eq( 'bookmark.user_id', $queryBuilder->createNamedParameter( - (int)$criterion->value[0], + (int)$value, ParameterType::INTEGER ) ); From 070e342c7fb0ed73f7961980a2bb1cbfff43b576 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 24 Feb 2026 09:38:07 +0100 Subject: [PATCH 04/29] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - PHPStand and Bookmark\IdSortClauseQueryBuilder --- phpstan-baseline.neon | 6 ------ .../Bookmark/IdSortClauseQueryBuilder.php | 9 ++++++++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f15029435a..016d3efe8a 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -22302,12 +22302,6 @@ parameters: count: 1 path: src/lib/Persistence/Legacy/Filter/Handler/LocationFilteringHandler.php - - - message: '#^PHPDoc tag @var with type Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Query\\SortClause is not subtype of native type Ibexa\\Contracts\\Core\\Repository\\Values\\Filter\\FilteringSortClause\.$#' - identifier: varTag.nativeType - count: 1 - path: src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php - - message: '#^PHPDoc tag @var with type Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Query\\SortClause is not subtype of native type Ibexa\\Contracts\\Core\\Repository\\Values\\Filter\\FilteringSortClause\.$#' identifier: varTag.nativeType diff --git a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php index 50e85f0858..1a0151f3dd 100644 --- a/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php @@ -24,7 +24,14 @@ public function buildQuery( FilteringQueryBuilder $queryBuilder, FilteringSortClause $sortClause ): void { - /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */ + if (!$sortClause instanceof BookmarkId) { + throw new \InvalidArgumentException(sprintf( + 'Expected %s, got %s', + BookmarkId::class, + get_class($sortClause), + )); + } + /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\BookmarkId $sortClause */ $queryBuilder->addSelect('bookmark.id'); $queryBuilder->addOrderBy('bookmark.id', $sortClause->direction); } From 245cba04ff82855d9707b16ba5e65470958d0712 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 24 Feb 2026 09:51:07 +0100 Subject: [PATCH 05/29] fixup! IBX-6773: Bookmarks for non-accessible contents cause exception - Added more Strict typing --- .../Values/Content/Query/Criterion/IsBookmarked.php | 10 +--------- src/lib/Repository/BookmarkService.php | 9 +++------ 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index 5ce8b9709c..1f14b5e726 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -20,15 +20,7 @@ */ final class IsBookmarked extends Criterion implements FilteringCriterion { - /** - * Creates a new IsBookmarked criterion. - * - * @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 - */ - public function __construct($value) + public function __construct(int $value) { parent::__construct(null, null, $value); } diff --git a/src/lib/Repository/BookmarkService.php b/src/lib/Repository/BookmarkService.php index 1ee28c7989..9a38e61018 100644 --- a/src/lib/Repository/BookmarkService.php +++ b/src/lib/Repository/BookmarkService.php @@ -26,14 +26,11 @@ class BookmarkService implements BookmarkServiceInterface { - /** @var \Ibexa\Contracts\Core\Repository\Repository */ - protected $repository; + protected RepositoryInterface $repository; - /** @var \Ibexa\Contracts\Core\Persistence\Bookmark\Handler */ - protected $bookmarkHandler; + protected BookmarkHandler $bookmarkHandler; - /** @var \Psr\Log\LoggerInterface */ - private $logger; + private LoggerInterface $logger; /** * BookmarkService constructor. From 96354746d82f6d81e2e8762053c396ced8d7f5e9 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 24 Feb 2026 09:53:39 +0100 Subject: [PATCH 06/29] Update tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php Co-authored-by: Konrad Oboza --- .../Location/BookmarkQueryBuilderTest.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php index b632ac35b2..8d62573154 100644 --- a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php +++ b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -8,7 +8,6 @@ namespace Ibexa\Tests\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location; -use Generator; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion as Criterion; use Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder; use Ibexa\Tests\Core\Persistence\Legacy\Filter\BaseCriterionVisitorQueryBuilderTestCase; @@ -16,7 +15,7 @@ final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTestCase { /** - * @return Generator}> + * @return iterable}> * * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidCriterionArgumentException */ From 6cc6903511d53ca091d7c42fc50d56109870407f Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Fri, 27 Feb 2026 21:03:38 +0100 Subject: [PATCH 07/29] [PHPStan] Aligned baseline with the upstream changes --- phpstan-baseline.neon | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 016d3efe8a..7ec2ff7bba 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -17706,6 +17706,12 @@ parameters: count: 1 path: src/lib/Persistence/Cache/UserPreferenceHandler.php + - + message: '#^Return type \(Doctrine\\ORM\\Mapping\\ClassMetadataFactory\) of method Ibexa\\Core\\Persistence\\Doctrine\\SiteAccessAwareEntityManager\:\:getMetadataFactory\(\) should be compatible with return type \(Doctrine\\Persistence\\Mapping\\ClassMetadataFactory\\>\) of method Doctrine\\Persistence\\ObjectManager\:\:getMetadataFactory\(\)$#' + identifier: method.childReturnType + count: 2 + path: src/lib/Persistence/Doctrine/SiteAccessAwareEntityManager.php + - message: '#^Property Ibexa\\Core\\Persistence\\FieldTypeRegistry\:\:\$coreFieldTypes \(array\\) does not accept array\\.$#' identifier: assign.propertyType @@ -25344,12 +25350,6 @@ parameters: count: 1 path: src/lib/Repository/TrashService.php - - - message: '#^Class Ibexa\\Contracts\\Core\\Persistence\\Content\\UrlAlias referenced with incorrect case\: Ibexa\\Contracts\\Core\\Persistence\\Content\\URLAlias\.$#' - identifier: class.nameCase - count: 2 - path: src/lib/Repository/URLAliasService.php - - message: '#^Method Ibexa\\Core\\Repository\\URLAliasService\:\:choosePrioritizedLanguageCode\(\) has parameter \$entries with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue From 0d293e542849ac0fd6c371ab9d68c4ee15a7e895 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 10 Mar 2026 10:39:03 +0100 Subject: [PATCH 08/29] Apply suggestions from code review Co-authored-by: Konrad Oboza --- src/lib/Repository/BookmarkService.php | 2 +- tests/integration/Core/Repository/LocationServiceTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/Repository/BookmarkService.php b/src/lib/Repository/BookmarkService.php index 9a38e61018..b432aa2356 100644 --- a/src/lib/Repository/BookmarkService.php +++ b/src/lib/Repository/BookmarkService.php @@ -111,7 +111,7 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList ->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC)) ->sliceBy($limit, $offset); - $result = $this->repository->getlocationService()->find($filter, []); + $result = $this->repository->getLocationService()->find($filter, []); } catch (BadStateException $e) { $this->logger->debug($e->getMessage(), [ 'exception' => $e, diff --git a/tests/integration/Core/Repository/LocationServiceTest.php b/tests/integration/Core/Repository/LocationServiceTest.php index dc38cdb5f3..ef4e505c1f 100644 --- a/tests/integration/Core/Repository/LocationServiceTest.php +++ b/tests/integration/Core/Repository/LocationServiceTest.php @@ -1990,8 +1990,8 @@ public function testBookmarksAreSwappedAfterSwapLocation() $afterSwap = $bookmarkService->loadBookmarks(); /* END: Use Case */ - $this->assertEquals($contactUsLocationId, $afterSwap->items[0]->id); - $this->assertEquals($beforeSwap->items[1]->id, $afterSwap->items[1]->id); + self::assertEquals($contactUsLocationId, $afterSwap->items[0]->id); + self::assertEquals($beforeSwap->items[1]->id, $afterSwap->items[1]->id); } /** From 8e63f7e47408c26d351315e6c03a831f1f678cd1 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 10 Mar 2026 12:57:56 +0100 Subject: [PATCH 09/29] Added integration test for using Criterion\IsBookmarked and LocationService::count() --- .../Core/Repository/BookmarkServiceTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/integration/Core/Repository/BookmarkServiceTest.php b/tests/integration/Core/Repository/BookmarkServiceTest.php index b4d1542d59..15887de5f1 100644 --- a/tests/integration/Core/Repository/BookmarkServiceTest.php +++ b/tests/integration/Core/Repository/BookmarkServiceTest.php @@ -10,6 +10,8 @@ use Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException; use Ibexa\Contracts\Core\Repository\Values\Bookmark\BookmarkList; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; +use Ibexa\Contracts\Core\Repository\Values\Filter\Filter; /** * Test case for the BookmarkService. @@ -150,6 +152,21 @@ public function testLoadBookmarks() return $location->id; }, $bookmarks->items)); } + + /** + * @covers \Ibexa\Contracts\Core\Repository\BookmarkService::loadBookmarks + */ + public function testCountBookmarks(): void + { + $repository = $this->getRepository(); + + $filter = new Filter(); + $filter + ->withCriterion(new Criterion\IsBookmarked(14)); + $count = $repository->getLocationService()->count($filter, []); + + $this->assertEquals($count, 5); + } } class_alias(BookmarkServiceTest::class, 'eZ\Publish\API\Repository\Tests\BookmarkServiceTest'); From 310d457f1f79065a0b6b799d45b23dde08b3356b Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 10 Mar 2026 13:05:39 +0100 Subject: [PATCH 10/29] fixup! Added integration test for using Criterion\IsBookmarked and LocationService::count() --- tests/integration/Core/Repository/BookmarkServiceTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/integration/Core/Repository/BookmarkServiceTest.php b/tests/integration/Core/Repository/BookmarkServiceTest.php index 15887de5f1..8bb43a8912 100644 --- a/tests/integration/Core/Repository/BookmarkServiceTest.php +++ b/tests/integration/Core/Repository/BookmarkServiceTest.php @@ -153,9 +153,6 @@ public function testLoadBookmarks() }, $bookmarks->items)); } - /** - * @covers \Ibexa\Contracts\Core\Repository\BookmarkService::loadBookmarks - */ public function testCountBookmarks(): void { $repository = $this->getRepository(); From 3594b4fdf1fb933bab362269e9d2e82979d1d662 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 10 Mar 2026 13:59:26 +0100 Subject: [PATCH 11/29] fixup! Added integration test for using Criterion\IsBookmarked and LocationService::count() --- phpstan-baseline.neon | 6 ------ tests/integration/Core/Repository/BookmarkServiceTest.php | 7 +++---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7ec2ff7bba..204c3d6511 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -32256,12 +32256,6 @@ parameters: count: 1 path: tests/integration/Core/Repository/BaseURLServiceTest.php - - - message: '#^Call to method PHPUnit\\Framework\\Assert\:\:assertInstanceOf\(\) with ''Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Values\\\\Bookmark\\\\BookmarkList'' and Ibexa\\Contracts\\Core\\Repository\\Values\\Bookmark\\BookmarkList will always evaluate to true\.$#' - identifier: method.alreadyNarrowedType - count: 1 - path: tests/integration/Core/Repository/BookmarkServiceTest.php - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\BookmarkServiceTest\:\:testCreateBookmark\(\) has no return type specified\.$#' identifier: missingType.return diff --git a/tests/integration/Core/Repository/BookmarkServiceTest.php b/tests/integration/Core/Repository/BookmarkServiceTest.php index 8bb43a8912..36624cb1e6 100644 --- a/tests/integration/Core/Repository/BookmarkServiceTest.php +++ b/tests/integration/Core/Repository/BookmarkServiceTest.php @@ -145,10 +145,9 @@ public function testLoadBookmarks() $bookmarks = $repository->getBookmarkService()->loadBookmarks(1, 3); /* END: Use Case */ - $this->assertInstanceOf(BookmarkList::class, $bookmarks); - $this->assertEquals($bookmarks->totalCount, 5); + self::assertEquals(5, $bookmarks->totalCount); // Assert bookmarks order: recently added should be first - $this->assertEquals([15, 13, 12], array_map(static function ($location) { + self::assertEquals([15, 13, 12], array_map(static function ($location) { return $location->id; }, $bookmarks->items)); } @@ -162,7 +161,7 @@ public function testCountBookmarks(): void ->withCriterion(new Criterion\IsBookmarked(14)); $count = $repository->getLocationService()->count($filter, []); - $this->assertEquals($count, 5); + $this->assertEquals(5, $count); } } From 5fb1a53cdbdf949db003f52ecc8ebcb6ed807f2b Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 10 Mar 2026 14:28:18 +0100 Subject: [PATCH 12/29] fixup! fixup! Added integration test for using Criterion\IsBookmarked and LocationService::count() --- tests/integration/Core/Repository/BookmarkServiceTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/Core/Repository/BookmarkServiceTest.php b/tests/integration/Core/Repository/BookmarkServiceTest.php index 36624cb1e6..8f714daaeb 100644 --- a/tests/integration/Core/Repository/BookmarkServiceTest.php +++ b/tests/integration/Core/Repository/BookmarkServiceTest.php @@ -9,7 +9,6 @@ namespace Ibexa\Tests\Integration\Core\Repository; use Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException; -use Ibexa\Contracts\Core\Repository\Values\Bookmark\BookmarkList; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use Ibexa\Contracts\Core\Repository\Values\Filter\Filter; From 6c862ded5dfeb3753d32cd3fd4b99f58ebb142fd Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 10 Mar 2026 14:41:33 +0100 Subject: [PATCH 13/29] Apply suggestion from @konradoboza Co-authored-by: Konrad Oboza --- tests/integration/Core/Repository/BookmarkServiceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/Core/Repository/BookmarkServiceTest.php b/tests/integration/Core/Repository/BookmarkServiceTest.php index 8f714daaeb..4182d45d62 100644 --- a/tests/integration/Core/Repository/BookmarkServiceTest.php +++ b/tests/integration/Core/Repository/BookmarkServiceTest.php @@ -160,7 +160,7 @@ public function testCountBookmarks(): void ->withCriterion(new Criterion\IsBookmarked(14)); $count = $repository->getLocationService()->count($filter, []); - $this->assertEquals(5, $count); + self::assertEquals(5, $count); } } From ca42c8d486dca54e25ce0f4e8ba99ba6881ec43d Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 10:43:14 +0200 Subject: [PATCH 14/29] Fixed deprecation notices --- src/contracts/Persistence/Bookmark/Handler.php | 4 ++-- src/lib/Persistence/Cache/BookmarkHandler.php | 4 ---- src/lib/Persistence/Legacy/Bookmark/Gateway.php | 4 ++-- .../Legacy/Bookmark/Gateway/DoctrineDatabase.php | 4 ---- .../Legacy/Bookmark/Gateway/ExceptionConversion.php | 8 -------- src/lib/Persistence/Legacy/Bookmark/Handler.php | 4 ---- 6 files changed, 4 insertions(+), 24 deletions(-) diff --git a/src/contracts/Persistence/Bookmark/Handler.php b/src/contracts/Persistence/Bookmark/Handler.php index 714d85a736..cf685b7cec 100644 --- a/src/contracts/Persistence/Bookmark/Handler.php +++ b/src/contracts/Persistence/Bookmark/Handler.php @@ -50,7 +50,7 @@ public function loadUserIdsByLocation(Location $location): array; /** * Loads bookmarks owned by user. * - * @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. + * @deprecated 4.6.30 The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. * * @param int $userId * @param int $offset the start offset for paging @@ -63,7 +63,7 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) /** * Count bookmarks owned by user. * - * @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. + * @deprecated 4.6.30 The "Handler::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. * * @param int $userId * diff --git a/src/lib/Persistence/Cache/BookmarkHandler.php b/src/lib/Persistence/Cache/BookmarkHandler.php index d8cab6d388..bf29f9454d 100644 --- a/src/lib/Persistence/Cache/BookmarkHandler.php +++ b/src/lib/Persistence/Cache/BookmarkHandler.php @@ -83,8 +83,6 @@ function (Bookmark $bookmark) { } /** - * @deprecated The "BookmarkHandler::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. - * * {@inheritdoc} */ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array @@ -99,8 +97,6 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated The "BookmarkHandler::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. - * * {@inheritdoc} */ public function countUserBookmarks(int $userId): int diff --git a/src/lib/Persistence/Legacy/Bookmark/Gateway.php b/src/lib/Persistence/Legacy/Bookmark/Gateway.php index e0c9220894..4905f253ab 100644 --- a/src/lib/Persistence/Legacy/Bookmark/Gateway.php +++ b/src/lib/Persistence/Legacy/Bookmark/Gateway.php @@ -52,7 +52,7 @@ abstract public function loadUserIdsByLocation(Location $location): array; /** * Load data for all bookmarks owned by given $userId. * - * @deprecated Gateway::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. + * @deprecated 4.6.30 Gateway::loadUserBookmarks()" method is deprecated, will be removed in 6.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 @@ -65,7 +65,7 @@ abstract public function loadUserBookmarks(int $userId, int $offset = 0, int $li /** * Count bookmarks owned by given $userId. * - * @deprecated The "Gateway::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. + * @deprecated 4.6.30 The "Gateway::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. * * @param int $userId ID of user * diff --git a/src/lib/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php b/src/lib/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php index fab5f7a23d..e30578885c 100644 --- a/src/lib/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php +++ b/src/lib/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php @@ -124,8 +124,6 @@ public function loadUserIdsByLocation(Location $location): array } /** - * @deprecated The "DoctrineDatabase::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. - * * {@inheritdoc} */ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array @@ -148,8 +146,6 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated The "DoctrineDatabase::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. - * * {@inheritdoc} */ public function countUserBookmarks(int $userId): int diff --git a/src/lib/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php b/src/lib/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php index 1371c20860..72bd954999 100644 --- a/src/lib/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php +++ b/src/lib/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php @@ -57,11 +57,6 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati } } - /** - * @deprecated The "ExceptionConversion::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. - * - * @return array - */ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array { try { @@ -71,9 +66,6 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } } - /** - * @deprecated The "ExceptionConversion::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. - */ public function countUserBookmarks(int $userId): int { try { diff --git a/src/lib/Persistence/Legacy/Bookmark/Handler.php b/src/lib/Persistence/Legacy/Bookmark/Handler.php index 91daed76e0..71d075fbae 100644 --- a/src/lib/Persistence/Legacy/Bookmark/Handler.php +++ b/src/lib/Persistence/Legacy/Bookmark/Handler.php @@ -75,8 +75,6 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra } /** - * @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead. - * * {@inheritdoc} */ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array @@ -87,8 +85,6 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1) } /** - * @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead. - * * {@inheritdoc} */ public function countUserBookmarks(int $userId): int From 2e5d7c79fec57281fa6757918f97dad83a902183 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 10:53:08 +0200 Subject: [PATCH 15/29] Removed dependency to Criterion --- .../Content/Query/Criterion/IsBookmarked.php | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index 1f14b5e726..afa9210051 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -9,26 +9,15 @@ namespace Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; -use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\Operator\Specifications; use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion; /** * A criterion that matches locations of bookmarks for a given user id. - * - * Supported operators: - * - EQ: matches against a unique user id */ -final class IsBookmarked extends Criterion implements FilteringCriterion +final class IsBookmarked implements FilteringCriterion { - public function __construct(int $value) - { - parent::__construct(null, null, $value); - } - - public function getSpecifications(): array - { - return [ - new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_INTEGER), - ]; + public function __construct( + public readonly int $value + ) { } } From c700f406d3c46150681f25dfc239f9fa4461fd98 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 12:31:40 +0200 Subject: [PATCH 16/29] Fixed so that Bookmark criterion has parameters isBookmarked and userId --- .../Content/Query/Criterion/IsBookmarked.php | 3 +- .../Location/BookmarkQueryBuilder.php | 58 ++++++++++++------- src/lib/Repository/BookmarkService.php | 4 +- 3 files changed, 39 insertions(+), 26 deletions(-) diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index afa9210051..d789092e34 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -17,7 +17,8 @@ final class IsBookmarked implements FilteringCriterion { public function __construct( - public readonly int $value + readonly bool $isBookmarked = true, + readonly ?int $userId = null, ) { } } diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index bf8f8712b9..067e89c59c 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -13,12 +13,19 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked; use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion; use Ibexa\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase; +use Ibexa\Core\Repository\Permission\PermissionResolver; /** * @internal for internal use by Repository Filtering */ final class BookmarkQueryBuilder extends BaseLocationCriterionQueryBuilder { + public function __construct( + private readonly PermissionResolver $permissionResolver, + ) { + + } + public function accepts(FilteringCriterion $criterion): bool { return $criterion instanceof IsBookmarked; @@ -28,30 +35,37 @@ public function buildQueryConstraint( FilteringQueryBuilder $queryBuilder, FilteringCriterion $criterion ): string { - $queryBuilder - ->joinOnce( - 'location', - DoctrineDatabase::TABLE_BOOKMARKS, - 'bookmark', - 'location.node_id = bookmark.node_id' - ); - /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */ - $value = $criterion->value; + $isBookmarked = $criterion->isBookmarked; + $userId = $criterion->userId ?? $this->permissionResolver->getCurrentUserReference()->getUserId(); - if (\is_array($value)) { - if (!isset($value[0])) { - throw new \InvalidArgumentException('IsBookmarked criterion value must contain userId at index 0.'); - } - $value = $value[0]; - } + if ( $isBookmarked ) { + $queryBuilder + ->joinOnce( + 'location', + DoctrineDatabase::TABLE_BOOKMARKS, + 'bookmark', + 'location.node_id = bookmark.node_id' + ); + + return $queryBuilder->expr()->eq( + 'bookmark.user_id', + $queryBuilder->createNamedParameter( + $userId, + ParameterType::INTEGER + ) + ); + } else { + $queryBuilder + ->leftJoinOnce( + 'location', + DoctrineDatabase::TABLE_BOOKMARKS, + 'bookmark', + 'location.node_id = bookmark.node_id AND bookmark.user_id = :userId' + ) + ->setParameter('userId', $userId); - return $queryBuilder->expr()->eq( - 'bookmark.user_id', - $queryBuilder->createNamedParameter( - (int)$value, - ParameterType::INTEGER - ) - ); + return $queryBuilder->expr()->isNull('bookmark.id'); + } } } diff --git a/src/lib/Repository/BookmarkService.php b/src/lib/Repository/BookmarkService.php index b432aa2356..102a71c098 100644 --- a/src/lib/Repository/BookmarkService.php +++ b/src/lib/Repository/BookmarkService.php @@ -102,12 +102,10 @@ public function deleteBookmark(Location $location): void */ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList { - $currentUserId = $this->getCurrentUserId(); - $filter = new Filter(); try { $filter - ->withCriterion(new Criterion\IsBookmarked($currentUserId)) + ->withCriterion(new Criterion\IsBookmarked()) ->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC)) ->sliceBy($limit, $offset); From 7368c0d22119c497fe1b0118fe33f3cd71ccb967 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 12:32:03 +0200 Subject: [PATCH 17/29] tests/integration/Core/Repository/BookmarkServiceTest.php Adjusted tests accoringly to latest commits --- .../Core/Repository/BookmarkServiceTest.php | 2 +- .../Location/BookmarkQueryBuilderTest.php | 18 +++++++----------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/tests/integration/Core/Repository/BookmarkServiceTest.php b/tests/integration/Core/Repository/BookmarkServiceTest.php index 4182d45d62..8c21ab8247 100644 --- a/tests/integration/Core/Repository/BookmarkServiceTest.php +++ b/tests/integration/Core/Repository/BookmarkServiceTest.php @@ -157,7 +157,7 @@ public function testCountBookmarks(): void $filter = new Filter(); $filter - ->withCriterion(new Criterion\IsBookmarked(14)); + ->withCriterion(new Criterion\IsBookmarked(true, 14)); $count = $repository->getLocationService()->count($filter, []); self::assertEquals(5, $count); diff --git a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php index 8d62573154..e499ddcd19 100644 --- a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php +++ b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -10,6 +10,7 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion as Criterion; use Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder; +use Ibexa\Core\Repository\Permission\PermissionResolver; use Ibexa\Tests\Core\Persistence\Legacy\Filter\BaseCriterionVisitorQueryBuilderTestCase; final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTestCase @@ -22,25 +23,20 @@ final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTes public function getFilteringCriteriaQueryData(): iterable { yield 'Bookmarks locations for user_id=14' => [ - new Criterion\IsBookmarked(14), + new Criterion\IsBookmarked(true, 14), 'bookmark.user_id = :dcValue1', ['dcValue1' => 14], ]; - yield 'Bookmarks locations for user_id=14 OR user_id=7' => [ - new Criterion\LogicalOr( - [ - new Criterion\IsBookmarked(14), - new Criterion\IsBookmarked(7), - ] - ), - '(bookmark.user_id = :dcValue1) OR (bookmark.user_id = :dcValue2)', - ['dcValue1' => 14, 'dcValue2' => 7], + yield 'Bookmarks locations for user_id=7' => [ + new Criterion\IsBookmarked(true, 7), + 'bookmark.user_id = :dcValue1', + ['dcValue1' => 7], ]; } protected function getCriterionQueryBuilders(): iterable { - return [new BookmarkQueryBuilder()]; + return [new BookmarkQueryBuilder($this->createMock(PermissionResolver::class))]; } } From abf0fff923c418ff77cbeccee977e0364d5d6122 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 12:44:44 +0200 Subject: [PATCH 18/29] Removed use of magic property --- src/lib/Repository/BookmarkService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Repository/BookmarkService.php b/src/lib/Repository/BookmarkService.php index 102a71c098..24bc5bea1b 100644 --- a/src/lib/Repository/BookmarkService.php +++ b/src/lib/Repository/BookmarkService.php @@ -120,7 +120,7 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList $list = new BookmarkList(); $list->totalCount = $result->totalCount; - $list->items = $result->locations; + $list->items = iterator_to_array($result->getIterator()); return $list; } From fb61a41cf7904ab9385afbd826948d708748b1cb Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 15:49:39 +0200 Subject: [PATCH 19/29] fixup! Fixed so that Bookmark criterion has parameters isBookmarked and userId --- .../Values/Content/Query/Criterion/IsBookmarked.php | 10 ++++++++-- .../Location/BookmarkQueryBuilder.php | 3 +-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index d789092e34..c083a37239 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -16,9 +16,15 @@ */ final class IsBookmarked implements FilteringCriterion { + public bool $isBookmarked = true; + + public ?int $userId = null; + public function __construct( - readonly bool $isBookmarked = true, - readonly ?int $userId = null, + bool $isBookmarked = true, + ?int $userId = null, ) { + $this->isBookmarked = $isBookmarked; + $this->userId = $userId; } } diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index 067e89c59c..5e38d11e53 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -23,7 +23,6 @@ final class BookmarkQueryBuilder extends BaseLocationCriterionQueryBuilder public function __construct( private readonly PermissionResolver $permissionResolver, ) { - } public function accepts(FilteringCriterion $criterion): bool @@ -39,7 +38,7 @@ public function buildQueryConstraint( $isBookmarked = $criterion->isBookmarked; $userId = $criterion->userId ?? $this->permissionResolver->getCurrentUserReference()->getUserId(); - if ( $isBookmarked ) { + if ($isBookmarked) { $queryBuilder ->joinOnce( 'location', From 690322ffdea63b370776f56b45855b17fe0bd278 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 16:30:49 +0200 Subject: [PATCH 20/29] Revert "Removed dependency to Criterion" This reverts commit 2e5d7c79fec57281fa6757918f97dad83a902183. --- .../Values/Content/Query/Criterion/IsBookmarked.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index c083a37239..ac58e4268f 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -9,12 +9,16 @@ namespace Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion; +use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\Operator\Specifications; use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion; /** * A criterion that matches locations of bookmarks for a given user id. + * + * Supported operators: + * - EQ: matches against a unique user id */ -final class IsBookmarked implements FilteringCriterion +final class IsBookmarked extends Criterion implements FilteringCriterion { public bool $isBookmarked = true; @@ -27,4 +31,11 @@ public function __construct( $this->isBookmarked = $isBookmarked; $this->userId = $userId; } + + public function getSpecifications(): array + { + return [ + new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_INTEGER), + ]; + } } From 4d3f4b78ef3d7ead374b24d4e508acdccaecee0a Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Tue, 21 Apr 2026 16:35:36 +0200 Subject: [PATCH 21/29] fixup! tests/integration/Core/Repository/BookmarkServiceTest.php --- .../Location/BookmarkQueryBuilderTest.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php index e499ddcd19..348625cba3 100644 --- a/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php +++ b/tests/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php @@ -28,8 +28,19 @@ public function getFilteringCriteriaQueryData(): iterable ['dcValue1' => 14], ]; + yield 'Bookmarks locations for user_id=14 OR user_id=7' => [ + new Criterion\LogicalOr( + [ + new Criterion\IsBookmarked(true, 14), + new Criterion\IsBookmarked(true, 7), + ] + ), + '(bookmark.user_id = :dcValue1) OR (bookmark.user_id = :dcValue2)', + ['dcValue1' => 14, 'dcValue2' => 7], + ]; + yield 'Bookmarks locations for user_id=7' => [ - new Criterion\IsBookmarked(true, 7), + new Criterion\IsBookmarked(true, 7), 'bookmark.user_id = :dcValue1', ['dcValue1' => 7], ]; From 80cb93e220a5462af941165ef12c3309d9c79741 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 22 Apr 2026 10:31:52 +0200 Subject: [PATCH 22/29] fixup! Revert "Removed dependency to Criterion" --- .../Values/Content/Query/Criterion/IsBookmarked.php | 8 +++----- .../Location/BookmarkQueryBuilder.php | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php index ac58e4268f..4d3c659956 100644 --- a/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php +++ b/src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php @@ -20,22 +20,20 @@ */ final class IsBookmarked extends Criterion implements FilteringCriterion { - public bool $isBookmarked = true; - public ?int $userId = null; public function __construct( bool $isBookmarked = true, - ?int $userId = null, + ?int $userId = null ) { - $this->isBookmarked = $isBookmarked; $this->userId = $userId; + parent::__construct(null, null, $isBookmarked); } public function getSpecifications(): array { return [ - new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_INTEGER), + new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_BOOLEAN), ]; } } diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index 5e38d11e53..a39b0a33b6 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -35,7 +35,7 @@ public function buildQueryConstraint( FilteringCriterion $criterion ): string { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */ - $isBookmarked = $criterion->isBookmarked; + $isBookmarked = $criterion->value; $userId = $criterion->userId ?? $this->permissionResolver->getCurrentUserReference()->getUserId(); if ($isBookmarked) { From 89d995e1d6a6c9a1f7705316102428d0970778ff Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 22 Apr 2026 12:17:08 +0200 Subject: [PATCH 23/29] fixup! Fixed so that Bookmark criterion has parameters isBookmarked and userId --- .../CriterionQueryBuilder/Location/BookmarkQueryBuilder.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index a39b0a33b6..7f493213b0 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -20,9 +20,12 @@ */ final class BookmarkQueryBuilder extends BaseLocationCriterionQueryBuilder { + private PermissionResolver $permissionResolver; + public function __construct( - private readonly PermissionResolver $permissionResolver, + PermissionResolver $permissionResolver ) { + $this->permissionResolver = $permissionResolver; } public function accepts(FilteringCriterion $criterion): bool From 6a32843dea77135ca527b2d72a7565dd96c662bd Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 22 Apr 2026 14:29:01 +0200 Subject: [PATCH 24/29] Fixed test Ibexa\Tests\Core\Repository\Service\Mock\BookmarkTest::testLoadBookmarks() --- tests/lib/Repository/Service/Mock/BookmarkTest.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/lib/Repository/Service/Mock/BookmarkTest.php b/tests/lib/Repository/Service/Mock/BookmarkTest.php index 876dc4a8e7..5bed42d23a 100644 --- a/tests/lib/Repository/Service/Mock/BookmarkTest.php +++ b/tests/lib/Repository/Service/Mock/BookmarkTest.php @@ -235,6 +235,11 @@ public function testLoadBookmarks() ->method('getLocationService') ->willReturn($locationServiceMock); + // All tests in this class expect a call to getPermissionResolver()->getCurrentUserReference(), except this very test + // This is because PermissionResolver is called from BookmarkQueryBuilder, not from BookmarkService when loading bookmarks + // As it is defined in setup() that getCurrentUserReference needs to be called at least once, we'll force a call here + $repository->getPermissionResolver()->getCurrentUserReference(); + $bookmarks = $this->createBookmarkService()->loadBookmarks($offset, $limit); $this->assertEquals($expectedTotalCount, $bookmarks->totalCount); From 2f69957e3f66f7db325a02cbbc69b0f5153c639a Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Wed, 22 Apr 2026 15:43:31 +0200 Subject: [PATCH 25/29] Fixed PHPstan error --- src/lib/Repository/BookmarkService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Repository/BookmarkService.php b/src/lib/Repository/BookmarkService.php index 24bc5bea1b..7c04108e87 100644 --- a/src/lib/Repository/BookmarkService.php +++ b/src/lib/Repository/BookmarkService.php @@ -38,7 +38,7 @@ class BookmarkService implements BookmarkServiceInterface * @param \Ibexa\Contracts\Core\Repository\Repository $repository * @param \Ibexa\Contracts\Core\Persistence\Bookmark\Handler $bookmarkHandler */ - public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler, LoggerInterface $logger = null) + public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler, ?LoggerInterface $logger = null) { $this->repository = $repository; $this->bookmarkHandler = $bookmarkHandler; From 83deac26887879de09245a252bdfc91475fb9c05 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 23 Apr 2026 09:27:54 +0200 Subject: [PATCH 26/29] fixup! fixup! Fixed so that Bookmark criterion has parameters isBookmarked and userId --- .../CriterionQueryBuilder/Location/BookmarkQueryBuilder.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index 7f493213b0..66c02c4eeb 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -38,7 +38,7 @@ public function buildQueryConstraint( FilteringCriterion $criterion ): string { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */ - $isBookmarked = $criterion->value; + $isBookmarked = $criterion->value[0]; $userId = $criterion->userId ?? $this->permissionResolver->getCurrentUserReference()->getUserId(); if ($isBookmarked) { From ba5cf53cd07312d78564dc8a3729c85156123ad4 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 23 Apr 2026 11:42:25 +0200 Subject: [PATCH 27/29] Added integration test for Criterion\IsBookmarked --- .../Filtering/Criterion/IsBookmarkedTest.php | 104 ++++++++++++++++++ 1 file changed, 104 insertions(+) create mode 100644 tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php diff --git a/tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php b/tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php new file mode 100644 index 0000000000..5f95e63a81 --- /dev/null +++ b/tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php @@ -0,0 +1,104 @@ +getRepository(false); + $locationService = $repository->getLocationService(); + + $baseFilter = new Filter(); + $totalCount = $locationService->count($baseFilter); + + $bookmarkedFilter = clone $baseFilter; + $bookmarkedFilter->withCriterion(new Criterion\IsBookmarked(true)); + $bookmarkedCount = $locationService->count($bookmarkedFilter); + + $notBookmarkedFilter = clone $baseFilter; + $notBookmarkedFilter->withCriterion(new Criterion\IsBookmarked(false)); + $notBookmarkedCount = $locationService->count($notBookmarkedFilter); + + self::assertSame( + $totalCount, + $bookmarkedCount + $notBookmarkedCount, + sprintf( + 'Mismatch: total=%d, bookmarked=%d, notBookmarked=%d', + $totalCount, + $bookmarkedCount, + $notBookmarkedCount + ) + ); + } + + public function isBookmarkedProvider(): iterable + { + // [isBookmarkedCriterion, initialCount, afterCreateCount, afterDeleteCount] + return [ + 'bookmarked=true' => [true, 0, 1, 0], + 'bookmarked=false' => [false, 1, 0, 1], + ]; + } + + /** + * @dataProvider isBookmarkedProvider + */ + public function testIsBookmarkedTrueAndFalse( + bool $isBookmarked, + int $initialCount, + int $afterCreateCount, + int $afterDeleteCount + ): void { + $repository = $this->getRepository(false); + $locationService = $repository->getLocationService(); + $bookmarkService = $repository->getBookmarkService(); + + $filesLocation = $locationService->loadLocation(52); + + $filter = new Filter(); + $filter->withCriterion(new Criterion\IsBookmarked($isBookmarked)) + ->andWithCriterion(new Criterion\LocationId(52)); + + $locations = $locationService->find($filter); + self::assertCount( + $initialCount, + $locations, + 'Unexpected initial bookmark state for IsBookmarked(' . ($isBookmarked ? 'true' : 'false') . ')' + ); + + $bookmarkService->createBookmark($filesLocation); + + $filter = new Filter(); + $filter->withCriterion(new Criterion\IsBookmarked($isBookmarked)) + ->andWithCriterion(new Criterion\LocationId(52)); + + $locations = $locationService->find($filter); + self::assertCount( + $afterCreateCount, + $locations, + 'Unexpected state after creating bookmark for IsBookmarked(' . ($isBookmarked ? 'true' : 'false') . ')' + ); + + $bookmarkService->deleteBookmark($filesLocation); + + $filter = new Filter(); + $filter->withCriterion(new Criterion\IsBookmarked($isBookmarked)) + ->andWithCriterion(new Criterion\LocationId(52)); + + $locations = $locationService->find($filter); + self::assertCount( + $afterDeleteCount, + $locations, + 'Unexpected state after deleting bookmark for IsBookmarked(' . ($isBookmarked ? 'true' : 'false') . ')' + ); + } +} From 4d5dc1e0b4c8c94263dd4fdf5f3d33a4ee343c30 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 23 Apr 2026 12:20:57 +0200 Subject: [PATCH 28/29] fixup! Added integration test for Criterion\IsBookmarked --- .../Core/Repository/Filtering/Criterion/IsBookmarkedTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php b/tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php index 5f95e63a81..c7024c7919 100644 --- a/tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php +++ b/tests/integration/Core/Repository/Filtering/Criterion/IsBookmarkedTest.php @@ -40,6 +40,9 @@ public function testBookmarkedAndNotBookmarkedCountsMatchTotal(): void ); } + /** + * @return iterable + */ public function isBookmarkedProvider(): iterable { // [isBookmarkedCriterion, initialCount, afterCreateCount, afterDeleteCount] From 0c025cdc4a363cb66fced17597e3d410d8805731 Mon Sep 17 00:00:00 2001 From: Vidar Langseid Date: Thu, 23 Apr 2026 12:46:59 +0200 Subject: [PATCH 29/29] fixup! fixup! Fixed so that Bookmark criterion has parameters isBookmarked and userId --- .../CriterionQueryBuilder/Location/BookmarkQueryBuilder.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php index 66c02c4eeb..d885c552ca 100644 --- a/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php +++ b/src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php @@ -38,7 +38,10 @@ public function buildQueryConstraint( FilteringCriterion $criterion ): string { /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */ - $isBookmarked = $criterion->value[0]; + $isBookmarked = $criterion->value[0] ?? null; + if (!is_bool($isBookmarked)) { + throw new \InvalidArgumentException('IsBookmarked criterion value must be boolean at index 0.'); + } $userId = $criterion->userId ?? $this->permissionResolver->getCurrentUserReference()->getUserId(); if ($isBookmarked) {