IBX:6773: Bookmarks for non accessible contents cause exception

phpstan-baseline.neon

@@ -18165,6 +18165,11 @@ parameters:
 			count: 1
 			path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/AncestorQueryBuilder.php
 
+		-
+			message: "#^Cannot access offset 0 on array\\<bool\\|float\\|int\\|string\\>\\|bool\\|float\\|int\\|string\\.$#"
+			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\\>\\|bool\\|float\\|int\\|string given\\.$#"
 			count: 1
@@ -57575,11 +57580,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\\.$#"
-			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\\.$#"
 			count: 1

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 5.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 5.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,42 @@
+<?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
+{
+    /**
+     * 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)
+    {
+        parent::__construct(null, null, $value);
+    }
+
+    public function getSpecifications(): array
+    {
+        return [
+            new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_INTEGER),
+        ];
+    }
+}

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/Cache/BookmarkHandler.php

@@ -83,6 +83,8 @@ function (Bookmark $bookmark) {
     }
 
     /**
+     * @deprecated The "BookmarkHandler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
+     *
      * {@inheritdoc}
      */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
@@ -97,6 +99,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
     }
 
     /**
+     * @deprecated The "BookmarkHandler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
+     *
      * {@inheritdoc}
      */
     public function countUserBookmarks(int $userId): int

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 Gateway::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
+     *
      * @param int $userId ID of user
      * @param int $offset Offset to start listing from, 0 by default
      * @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 The "Gateway::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
+     *
      * @param int $userId ID of user
      *
      * @return int

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

@@ -124,6 +124,8 @@ public function loadUserIdsByLocation(Location $location): array
     }
 
     /**
+     * @deprecated The "DoctrineDatabase::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
+     *
      * {@inheritdoc}
      */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
@@ -146,6 +148,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
     }
 
     /**
+     * @deprecated The "DoctrineDatabase::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
+     *
      * {@inheritdoc}
      */
     public function countUserBookmarks(int $userId): int

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

@@ -57,6 +57,11 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati
         }
     }
 
+    /**
+     * @deprecated The "ExceptionConversion::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
+     *
+     * @return array
+     */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
     {
         try {
@@ -66,6 +71,9 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
         }
     }
 
+    /**
+     * @deprecated The "ExceptionConversion::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
+     */
     public function countUserBookmarks(int $userId): int
     {
         try {

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

@@ -75,6 +75,8 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra
     }
 
     /**
+     * @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
+     *
      * {@inheritdoc}
      */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
@@ -85,6 +87,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
     }
 
     /**
+     * @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
+     *
      * {@inheritdoc}
      */
     public function countUserBookmarks(int $userId): int

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

@@ -0,0 +1,48 @@
+<?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;
+
+/**
+ * @internal for internal use by Repository Filtering
+ */
+final class BookmarkQueryBuilder extends BaseLocationCriterionQueryBuilder
+{
+    public function accepts(FilteringCriterion $criterion): bool
+    {
+        return $criterion instanceof IsBookmarked;
+    }
+
+    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 */
+        return $queryBuilder->expr()->eq(
+            'bookmark.user_id',
+            $queryBuilder->createNamedParameter(
+                (int)$criterion->value[0],
+                ParameterType::INTEGER
+            )
+        );
+    }
+}

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

@@ -0,0 +1,31 @@
+<?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 {
+        /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
+        $queryBuilder->addSelect('bookmark.id');
+        $queryBuilder->addOrderBy('bookmark.id', $sortClause->direction);
+    }
+}

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

src/lib/Repository/Repository.php

@@ -602,7 +602,8 @@ public function getBookmarkService(): BookmarkServiceInterface
         if ($this->bookmarkService === null) {
             $this->bookmarkService = new BookmarkService(
                 $this,
-                $this->persistenceHandler->bookmarkHandler()
+                $this->persistenceHandler->bookmarkHandler(),
+                $this->logger
             );
         }