IBX-6773: Bookmarks for non accessible contents cause exception

eZ/Publish/API/Repository/Tests/LocationServiceTest.php

@@ -1965,13 +1965,15 @@ public function testBookmarksAreSwappedAfterSwapLocation()
 
         $mediaLocationId = $this->generateId('location', 43);
         $demoDesignLocationId = $this->generateId('location', 56);
+        $contactUsLocationId = $this->generateId('location', 60);
 
         /* BEGIN: Use Case */
         $locationService = $repository->getLocationService();
         $bookmarkService = $repository->getBookmarkService();
 
         $mediaLocation = $locationService->loadLocation($mediaLocationId);
         $demoDesignLocation = $locationService->loadLocation($demoDesignLocationId);
+        $contactUsLocation = $locationService->loadLocation($contactUsLocationId);
 
         // Bookmark locations
         $bookmarkService->createBookmark($mediaLocation);
@@ -1980,13 +1982,24 @@ public function testBookmarksAreSwappedAfterSwapLocation()
         $beforeSwap = $bookmarkService->loadBookmarks();
 
         // Swaps the content referred to by the locations
-        $locationService->swapLocation($mediaLocation, $demoDesignLocation);
+        $locationService->swapLocation($demoDesignLocation, $contactUsLocation);
 
         $afterSwap = $bookmarkService->loadBookmarks();
         /* END: Use Case */
 
-        $this->assertEquals($beforeSwap->items[0]->id, $afterSwap->items[1]->id);
-        $this->assertEquals($beforeSwap->items[1]->id, $afterSwap->items[0]->id);
+        $expectedIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) {
+            if ($item->id === $demoDesignLocationId) {
+                return $contactUsLocationId;
+            }
+
+            return $item->id;
+        }, $beforeSwap->items);
+
+        $actualIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) {
+            return $item->id;
+        }, $afterSwap->items);
+
+        $this->assertEquals($expectedIdsAfter, $actualIdsAfter);
     }
 
     /**

eZ/Publish/API/Repository/Values/Content/Query/Criterion/IsBookmarked.php

@@ -0,0 +1,43 @@
+<?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 eZ\Publish\API\Repository\Values\Content\Query\Criterion;
+
+use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
+use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Operator\Specifications;
+use eZ\Publish\SPI\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
+ */
+class IsBookmarked extends Criterion implements FilteringCriterion
+{
+    /**
+     * Creates a new IsBookmarked criterion.
+     *
+     * @param int $value UserID 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),
+        ];
+    }
+}

eZ/Publish/API/Repository/Values/Content/Query/SortClause/BookmarkId.php

@@ -0,0 +1,29 @@
+<?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 eZ\Publish\API\Repository\Values\Content\Query\SortClause;
+
+use eZ\Publish\API\Repository\Values\Content\Query;
+use eZ\Publish\API\Repository\Values\Content\Query\SortClause;
+use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause;
+
+/**
+ * Sets sort direction on the Bookmark ID for a location query containing a IsBookmarked criterion.
+ */
+class BookmarkId extends SortClause implements FilteringSortClause
+{
+    /**
+     * Constructs a new BookmarkId SortClause.
+     *
+     * @param string $sortDirection
+     */
+    public function __construct(string $sortDirection = Query::SORT_ASC)
+    {
+        parent::__construct('id', $sortDirection);
+    }
+}

eZ/Publish/Core/Persistence/Cache/BookmarkHandler.php

@@ -85,6 +85,8 @@ function (Bookmark $bookmark) {
     }
 
     /**
+     * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead.
+     *
      * {@inheritdoc}
      */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
@@ -99,6 +101,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
     }
 
     /**
+     * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead.
+     *
      * {@inheritdoc}
      */
     public function countUserBookmarks(int $userId): int

eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway.php

@@ -44,6 +44,8 @@ abstract public function loadBookmarkDataByUserIdAndLocationId(int $userId, arra
     /**
      * Load data for all bookmarks owned by given $userId.
      *
+     * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead.
+     *
      * @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)
@@ -55,6 +57,8 @@ abstract public function loadUserBookmarks(int $userId, int $offset = 0, int $li
     /**
      * Count bookmarks owned by given $userId.
      *
+     * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead.
+     *
      * @param int $userId ID of user
      *
      * @return int

eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/DoctrineDatabase.php

@@ -101,6 +101,8 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati
     }
 
     /**
+     * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead.
+     *
      * {@inheritdoc}
      */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
@@ -123,6 +125,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
     }
 
     /**
+     * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead.
+     *
      * {@inheritdoc}
      */
     public function countUserBookmarks(int $userId): int

eZ/Publish/Core/Persistence/Legacy/Bookmark/Gateway/ExceptionConversion.php

@@ -56,6 +56,15 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati
         }
     }
 
+    /**
+     * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead.
+     *
+     * @param int $userId
+     * @param int $offset
+     * @param int $limit
+     *
+     * @return array
+     */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
     {
         try {
@@ -65,6 +74,13 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
         }
     }
 
+    /**
+     * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead.
+     *
+     * @param int $userId
+     *
+     * @return int
+     */
     public function countUserBookmarks(int $userId): int
     {
         try {

eZ/Publish/Core/Persistence/Legacy/Bookmark/Handler.php

@@ -74,6 +74,8 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra
     }
 
     /**
+     * @deprecated Please use LocationService::find() and Criterion\IsBookmarked instead.
+     *
      * {@inheritdoc}
      */
     public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
@@ -84,6 +86,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
     }
 
     /**
+     * @deprecated Please use LocationService::count() and Criterion\IsBookmarked instead.
+     *
      * {@inheritdoc}
      */
     public function countUserBookmarks(int $userId): int

eZ/Publish/Core/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php

@@ -0,0 +1,47 @@
+<?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 eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location;
+
+use Doctrine\DBAL\ParameterType;
+use eZ\Publish\API\Repository\Values\Content\Query\Criterion\IsBookmarked;
+use eZ\Publish\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase;
+use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder;
+use eZ\Publish\SPI\Repository\Values\Filter\FilteringCriterion;
+
+/**
+ * @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'
+            );
+
+        return $queryBuilder->expr()->eq(
+            'bookmark.user_id',
+            $queryBuilder->createNamedParameter(
+                (int)$criterion->value[0],
+                ParameterType::INTEGER
+            )
+        );
+    }
+}

eZ/Publish/Core/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 eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark;
+
+use eZ\Publish\API\Repository\Values\Content\Query\SortClause\BookmarkId;
+use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder;
+use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause;
+use eZ\Publish\SPI\Repository\Values\Filter\SortClauseQueryBuilder;
+
+class IdSortClauseQueryBuilder implements SortClauseQueryBuilder
+{
+    public function accepts(FilteringSortClause $sortClause): bool
+    {
+        return $sortClause instanceof BookmarkId;
+    }
+
+    public function buildQuery(
+        FilteringQueryBuilder $queryBuilder,
+        FilteringSortClause $sortClause
+    ): void {
+        /** @var \eZ\Publish\API\Repository\Values\Content\Query\SortClause $sortClause */
+        $queryBuilder->addSelect('bookmark.id');
+        $queryBuilder->addOrderBy('bookmark.id', $sortClause->direction);
+    }
+}

eZ/Publish/Core/Persistence/Legacy/Tests/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilderTest.php

@@ -0,0 +1,45 @@
+<?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 eZ\Publish\Core\Persistence\Legacy\Tests\Filter\CriterionQueryBuilder\Location;
+
+use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
+use eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder;
+use eZ\Publish\Core\Persistence\Legacy\Tests\Filter\BaseCriterionVisitorQueryBuilderTestCase;
+
+/**
+ * @covers \eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder::buildQueryConstraint
+ * @covers \eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder::accepts
+ */
+final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTestCase
+{
+    public function getFilteringCriteriaQueryData(): iterable
+    {
+        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()];
+    }
+}

eZ/Publish/Core/Repository/BookmarkService.php

@@ -13,8 +13,11 @@
 use eZ\Publish\API\Repository\Repository as RepositoryInterface;
 use eZ\Publish\API\Repository\Values\Bookmark\BookmarkList;
 use eZ\Publish\API\Repository\Values\Content\Location;
+use eZ\Publish\API\Repository\Values\Content\Query;
+use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
+use eZ\Publish\API\Repository\Values\Content\Query\SortClause;
+use eZ\Publish\API\Repository\Values\Filter\Filter;
 use eZ\Publish\Core\Base\Exceptions\InvalidArgumentException;
-use eZ\Publish\SPI\Persistence\Bookmark\Bookmark;
 use eZ\Publish\SPI\Persistence\Bookmark\CreateStruct;
 use eZ\Publish\SPI\Persistence\Bookmark\Handler as BookmarkHandler;
 
@@ -97,16 +100,22 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList
     {
         $currentUserId = $this->getCurrentUserId();
 
-        $list = new BookmarkList();
-        $list->totalCount = $this->bookmarkHandler->countUserBookmarks($currentUserId);
-        if ($list->totalCount > 0) {
-            $bookmarks = $this->bookmarkHandler->loadUserBookmarks($currentUserId, $offset, $limit);
-
-            $list->items = array_map(function (Bookmark $bookmark) {
-                return $this->repository->getLocationService()->loadLocation($bookmark->locationId);
-            }, $bookmarks);
+        $filter = new Filter();
+        try {
+            $filter
+                ->withCriterion(new Criterion\IsBookmarked($currentUserId))
+                ->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC))
+                ->sliceBy($limit, $offset);
+
+            $result = $this->repository->getlocationService()->find($filter, []);
+        } catch (\eZ\Publish\API\Repository\Exceptions\BadStateException $e) {
+            return new BookmarkList();
         }
 
+        $list = new BookmarkList();
+        $list->totalCount = $result->totalCount;
+        $list->items = $result->locations;
+
         return $list;
     }