IBX-6773: Fixed loading Bookmarks for non-accessible content items

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
@@ -69570,12 +69564,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

src/contracts/Persistence/Bookmark/Handler.php

@@ -50,6 +50,8 @@ public function loadUserIdsByLocation(Location $location): array;
     /**
      * Loads bookmarks owned by user.
      *
+     * @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
      * @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 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
      *
      * @return int

src/contracts/Repository/Values/Content/Query/Criterion/IsBookmarked.php

@@ -0,0 +1,39 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+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
+{
+    public ?int $userId = null;
+
+    public function __construct(
+        bool $isBookmarked = true,
+        ?int $userId = null
+    ) {
+        $this->userId = $userId;
+        parent::__construct(null, null, $isBookmarked);
+    }
+
+    public function getSpecifications(): array
+    {
+        return [
+            new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_BOOLEAN),
+        ];
+    }
+}

src/contracts/Repository/Values/Content/Query/SortClause/BookmarkId.php

@@ -0,0 +1,24 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause;
+
+use Ibexa\Contracts\Core\Repository\Values\Content\Query;
+use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause;
+use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause;
+
+/**
+ * Sets sort direction on the bookmark id for a location query containing IsBookmarked criterion.
+ */
+final class BookmarkId extends SortClause implements FilteringSortClause
+{
+    public function __construct(string $sortDirection = Query::SORT_ASC)
+    {
+        parent::__construct('id', $sortDirection);
+    }
+}

src/lib/Persistence/Legacy/Bookmark/Gateway.php

@@ -52,6 +52,8 @@ abstract public function loadUserIdsByLocation(Location $location): array;
     /**
      * Load data for all bookmarks owned by given $userId.
      *
+     * @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
      * @param int $limit Limit for the listing. -1 by default (no limit)
@@ -63,6 +65,8 @@ abstract public function loadUserBookmarks(int $userId, int $offset = 0, int $li
     /**
      * Count bookmarks owned by given $userId.
      *
+     * @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
      *
      * @return int

src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php

@@ -0,0 +1,76 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location;
+
+use Doctrine\DBAL\ParameterType;
+use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
+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
+{
+    private PermissionResolver $permissionResolver;
+
+    public function __construct(
+        PermissionResolver $permissionResolver
+    ) {
+        $this->permissionResolver = $permissionResolver;
+    }
+
+    public function accepts(FilteringCriterion $criterion): bool
+    {
+        return $criterion instanceof IsBookmarked;
+    }
+
+    public function buildQueryConstraint(
+        FilteringQueryBuilder $queryBuilder,
+        FilteringCriterion $criterion
+    ): string {
+        /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */
+        $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) {
+            $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()->isNull('bookmark.id');
+        }
+    }
+}

src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php

@@ -0,0 +1,38 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark;
+
+use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
+use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\BookmarkId;
+use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause;
+use Ibexa\Contracts\Core\Repository\Values\Filter\SortClauseQueryBuilder;
+
+final class IdSortClauseQueryBuilder implements SortClauseQueryBuilder
+{
+    public function accepts(FilteringSortClause $sortClause): bool
+    {
+        return $sortClause instanceof BookmarkId;
+    }
+
+    public function buildQuery(
+        FilteringQueryBuilder $queryBuilder,
+        FilteringSortClause $sortClause
+    ): void {
+        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);
+    }
+}

src/lib/Repository/BookmarkService.php

@@ -9,33 +9,40 @@
 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
 {
-    /** @var \Ibexa\Contracts\Core\Repository\Repository */
-    protected $repository;
+    protected RepositoryInterface $repository;
 
-    /** @var \Ibexa\Contracts\Core\Persistence\Bookmark\Handler */
-    protected $bookmarkHandler;
+    protected BookmarkHandler $bookmarkHandler;
+
+    private LoggerInterface $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();
     }
 
     /**
@@ -95,17 +102,25 @@ 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())
+                ->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 = $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);
-        }
+        $list->totalCount = $result->totalCount;
+        $list->items = iterator_to_array($result->getIterator());
 
         return $list;
     }

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
             );
         }
 

tests/integration/Core/Repository/BookmarkServiceTest.php

@@ -9,7 +9,8 @@
 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;
 
 /**
  * Test case for the BookmarkService.
@@ -143,13 +144,24 @@ 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));
     }
+
+    public function testCountBookmarks(): void
+    {
+        $repository = $this->getRepository();
+
+        $filter = new Filter();
+        $filter
+            ->withCriterion(new Criterion\IsBookmarked(true, 14));
+        $count = $repository->getLocationService()->count($filter, []);
+
+        self::assertEquals(5, $count);
+    }
 }
 
 class_alias(BookmarkServiceTest::class, 'eZ\Publish\API\Repository\Tests\BookmarkServiceTest');