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

phpstan-baseline.neon

@@ -32220,12 +32220,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
@@ -69499,12 +69493,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.32 The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\Location\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.32 The "Handler::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\Location\IsBookmarked" instead.
+     *
      * @param int $userId
      *
      * @return int

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

@@ -15,6 +15,8 @@
 
 /**
  * This criterion only works for current user reference.
+ *
+ * When used with Content filtering, it matches bookmarks placed on main Locations only.
  */
 final class IsBookmarked extends Location implements FilteringCriterion
 {

src/contracts/Repository/Values/Content/Query/SortClause/Location/Bookmark/Id.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\Location\Bookmark;
+
+use Ibexa\Contracts\Core\Repository\Values\Content\Query;
+use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\Location;
+use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause;
+
+/**
+ * Sets sort direction on the bookmark id for a location query containing IsBookmarked criterion.
+ */
+final class Id extends Location 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.32 "Gateway::loadUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::find()" and "Criterion\Location\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.32 The "Gateway::countUserBookmarks()" method is deprecated, will be removed in 6.0.0. Use "LocationService::count()" and "Criterion\Location\IsBookmarked" instead.
+     *
      * @param int $userId ID of user
      *
      * @return int

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

@@ -0,0 +1,81 @@
+<?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 Doctrine\DBAL\Query\QueryBuilder;
+use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
+use Ibexa\Contracts\Core\Repository\PermissionResolver;
+use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\Location\IsBookmarked;
+use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion;
+use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
+use Ibexa\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase;
+
+/**
+ * @internal for internal use by Repository Filtering
+ */
+final class IsBookmarkedQueryBuilder extends BaseLocationCriterionQueryBuilder
+{
+    private const ALIAS = 'bookmark';
+
+    private PermissionResolver $permissionResolver;
+
+    public function __construct(
+        PermissionResolver $permissionResolver
+    ) {
+        $this->permissionResolver = $permissionResolver;
+    }
+
+    public function accepts(FilteringCriterion $criterion): bool
+    {
+        return $criterion instanceof IsBookmarked;
+    }
+
+    /**
+     * @param \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\Location\IsBookmarked $criterion
+     *
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
+     */
+    public function buildQueryConstraint(
+        FilteringQueryBuilder $queryBuilder,
+        FilteringCriterion $criterion
+    ): string {
+        parent::buildQueryConstraint($queryBuilder, $criterion);
+
+        $isBookmarked = $criterion->value[0] ?? null;
+        if (!is_bool($isBookmarked)) {
+            throw new InvalidArgumentException(
+                '$criterion',
+                'IsBookmarked criterion value must be boolean at index 0.'
+            );
+        }
+
+        $userId = $this->permissionResolver->getCurrentUserReference()->getUserId();
+
+        $subQueryBuilder = new QueryBuilder($queryBuilder->getConnection());
+        $subQueryBuilder
+            ->select('1')
+            ->from(DoctrineDatabase::TABLE_BOOKMARKS, self::ALIAS)
+            ->where(
+                $subQueryBuilder->expr()->eq(
+                    self::ALIAS . '.' . DoctrineDatabase::COLUMN_USER_ID,
+                    $queryBuilder->createNamedParameter($userId, ParameterType::INTEGER)
+                ),
+                $subQueryBuilder->expr()->eq(
+                    self::ALIAS . '.' . DoctrineDatabase::COLUMN_LOCATION_ID,
+                    'location.node_id'
+                )
+            );
+
+        return sprintf(
+            $isBookmarked ? 'EXISTS (%s)' : 'NOT EXISTS (%s)',
+            $subQueryBuilder->getSQL()
+        );
+    }
+}

src/lib/Persistence/Legacy/Filter/SortClauseQueryBuilder/Location/BaseLocationSortClauseQueryBuilder.php

@@ -28,18 +28,29 @@ public function buildQuery(
         $locationContext = $this->prepareLocationContext($queryBuilder);
         $locationAlias = $locationContext['alias'];
 
-        $sort = $this->getSortingExpressionForAlias($locationAlias);
-        $sortAlias = $this->getSortFieldAlias($sort);
-        $queryBuilder->addSelect(sprintf('%s AS %s', $sort, $sortAlias));
-
         if ($locationContext['needsMainLocationJoin']) {
             $this->joinMainLocationOnly($queryBuilder, $locationAlias);
         }
 
+        $this->joinAdditionalTables($queryBuilder, $locationAlias);
+
+        $sort = $this->getSortingExpressionForAlias($locationAlias);
+        $sortAlias = $this->getSortFieldAlias($sort);
+        $queryBuilder->addSelect(sprintf('%s AS %s', $sort, $sortAlias));
+
         /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
         $queryBuilder->addOrderBy($sortAlias, $sortClause->direction);
     }
 
+    /**
+     * Hook for sort clauses which need to join further tables against the resolved Location alias.
+     */
+    protected function joinAdditionalTables(
+        FilteringQueryBuilder $queryBuilder,
+        string $locationAlias
+    ): void {
+    }
+
     /**
      * @return array{alias: string, needsMainLocationJoin: bool}
      */

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

@@ -0,0 +1,95 @@
+<?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\Location\Bookmark;
+
+use Doctrine\DBAL\ParameterType;
+use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
+use Ibexa\Contracts\Core\Repository\PermissionResolver;
+use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\Location\Bookmark\Id;
+use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause;
+use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
+use Ibexa\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase;
+use Ibexa\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Location\BaseLocationSortClauseQueryBuilder;
+
+/**
+ * @internal
+ */
+final class IdSortClauseQueryBuilder extends BaseLocationSortClauseQueryBuilder
+{
+    private const ALIAS = 'ibexa_sort_bookmark';
+
+    private PermissionResolver $permissionResolver;
+
+    public function __construct(PermissionResolver $permissionResolver)
+    {
+        $this->permissionResolver = $permissionResolver;
+    }
+
+    public function accepts(FilteringSortClause $sortClause): bool
+    {
+        return $sortClause instanceof Id;
+    }
+
+    /**
+     * @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
+     */
+    public function buildQuery(
+        FilteringQueryBuilder $queryBuilder,
+        FilteringSortClause $sortClause
+    ): void {
+        if (!$sortClause instanceof Id) {
+            throw new InvalidArgumentException(
+                '$sortClause',
+                sprintf('Expected %s, got %s', Id::class, get_class($sortClause))
+            );
+        }
+
+        parent::buildQuery($queryBuilder, $sortClause);
+    }
+
+    protected function joinAdditionalTables(
+        FilteringQueryBuilder $queryBuilder,
+        string $locationAlias
+    ): void {
+        $userId = $this->permissionResolver->getCurrentUserReference()->getUserId();
+
+        $queryBuilder->leftJoinOnce(
+            $locationAlias,
+            DoctrineDatabase::TABLE_BOOKMARKS,
+            self::ALIAS,
+            (string)$queryBuilder->expr()->and(
+                sprintf(
+                    '%s.node_id = %s.%s',
+                    $locationAlias,
+                    self::ALIAS,
+                    DoctrineDatabase::COLUMN_LOCATION_ID
+                ),
+                $queryBuilder->expr()->eq(
+                    sprintf('%s.%s', self::ALIAS, DoctrineDatabase::COLUMN_USER_ID),
+                    $queryBuilder->createNamedParameter($userId, ParameterType::INTEGER)
+                )
+            )
+        );
+    }
+
+    protected function getSortingExpression(): string
+    {
+        return self::ALIAS . '.' . DoctrineDatabase::COLUMN_ID;
+    }
+
+    protected function getSortingExpressionForAlias(string $locationAlias): string
+    {
+        return $this->getSortingExpression();
+    }
+
+    protected function getSortFieldName(string $sortExpression): string
+    {
+        return 'bookmark_id';
+    }
+}

src/lib/Repository/BookmarkService.php

@@ -9,33 +9,34 @@
 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\Exception as RepositoryException;
 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;
 
-    /**
-     * BookmarkService constructor.
-     *
-     * @param \Ibexa\Contracts\Core\Repository\Repository $repository
-     * @param \Ibexa\Contracts\Core\Persistence\Bookmark\Handler $bookmarkHandler
-     */
-    public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler)
+    private LoggerInterface $logger;
+
+    public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler, ?LoggerInterface $logger = null)
     {
         $this->repository = $repository;
         $this->bookmarkHandler = $bookmarkHandler;
+        $this->logger = $logger ?? new NullLogger();
     }
 
     /**
@@ -95,17 +96,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\Location\IsBookmarked())
+                ->withSortClause(new SortClause\Location\Bookmark\Id(Query::SORT_DESC))
+                ->sliceBy($limit, $offset);
+
+            $result = $this->repository->getLocationService()->find($filter, []);
+        } catch (RepositoryException $e) {
+            $this->logger->error($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
             );
         }