Skip to content

Commit ddca556

Browse files
vidarlvidarl
committed
IBX-6773: Bookmarks for non-accessible contents cause exception
1 parent 98b7b50 commit ddca556

File tree

14 files changed

+270
-52
lines changed

14 files changed

+270
-52
lines changed

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

+16-3
Original file line numberDiff line numberDiff line change
@@ -1965,13 +1965,15 @@ public function testBookmarksAreSwappedAfterSwapLocation()
19651965

19661966
$mediaLocationId = $this->generateId('location', 43);
19671967
$demoDesignLocationId = $this->generateId('location', 56);
1968+
$contactUsLocationId = $this->generateId('location', 60);
19681969

19691970
/* BEGIN: Use Case */
19701971
$locationService = $repository->getLocationService();
19711972
$bookmarkService = $repository->getBookmarkService();
19721973

19731974
$mediaLocation = $locationService->loadLocation($mediaLocationId);
19741975
$demoDesignLocation = $locationService->loadLocation($demoDesignLocationId);
1976+
$contactUsLocation = $locationService->loadLocation($contactUsLocationId);
19751977

19761978
// Bookmark locations
19771979
$bookmarkService->createBookmark($mediaLocation);
@@ -1980,13 +1982,24 @@ public function testBookmarksAreSwappedAfterSwapLocation()
19801982
$beforeSwap = $bookmarkService->loadBookmarks();
19811983

19821984
// Swaps the content referred to by the locations
1983-
$locationService->swapLocation($mediaLocation, $demoDesignLocation);
1985+
$locationService->swapLocation($demoDesignLocation, $contactUsLocation);
19841986

19851987
$afterSwap = $bookmarkService->loadBookmarks();
19861988
/* END: Use Case */
19871989

1988-
$this->assertEquals($beforeSwap->items[0]->id, $afterSwap->items[1]->id);
1989-
$this->assertEquals($beforeSwap->items[1]->id, $afterSwap->items[0]->id);
1990+
$expectedIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) {
1991+
if ($item->id === $demoDesignLocationId) {
1992+
return $contactUsLocationId;
1993+
}
1994+
1995+
return $item->id;
1996+
}, $beforeSwap->items);
1997+
1998+
$actualIdsAfter = array_map(static function (Location $item) use ($demoDesignLocationId, $contactUsLocationId) {
1999+
return $item->id;
2000+
}, $afterSwap->items);
2001+
2002+
$this->assertEquals($expectedIdsAfter, $actualIdsAfter);
19902003
}
19912004

19922005
/**

‎eZ/Publish/API/Repository/Values/Content/Query/Criterion/Bookmark.php

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace eZ\Publish\API\Repository\Values\Content\Query\Criterion;
10+
11+
use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
12+
use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Operator\Specifications;
13+
use eZ\Publish\SPI\Repository\Values\Filter\FilteringCriterion;
14+
15+
/**
16+
* A criterion that matches locations of bookmarks for a given user id.
17+
*
18+
*
19+
* Supported operators:
20+
* - EQ: matches against a unique user id
21+
*/
22+
class Bookmark extends Criterion implements FilteringCriterion
23+
{
24+
/**
25+
* Creates a new Bookmark criterion.
26+
*
27+
* @param int $value UserID for which bookmarked locations must be matched against
28+
*
29+
* @throws \InvalidArgumentException if a non numeric id is given
30+
* @throws \InvalidArgumentException if the value type doesn't match the operator
31+
*/
32+
public function __construct($value)
33+
{
34+
parent::__construct(null, null, $value);
35+
}
36+
37+
public function getSpecifications(): array
38+
{
39+
return [
40+
new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_INTEGER),
41+
];
42+
}
43+
}

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

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace eZ\Publish\API\Repository\Values\Content\Query\SortClause;
10+
11+
use eZ\Publish\API\Repository\Values\Content\Query;
12+
use eZ\Publish\API\Repository\Values\Content\Query\SortClause;
13+
use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause;
14+
15+
/**
16+
* Sets sort direction on the bookmark id for a location query containing a Bookmark criterion.
17+
*/
18+
class BookmarkId extends SortClause implements FilteringSortClause
19+
{
20+
/**
21+
* Constructs a new BookmarkId SortClause.
22+
*
23+
* @param string $sortDirection
24+
*/
25+
public function __construct(string $sortDirection = Query::SORT_ASC)
26+
{
27+
parent::__construct('id', $sortDirection);
28+
}
29+
}

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

+4
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ function (Bookmark $bookmark) {
8585
}
8686

8787
/**
88+
* @deprecated Please use LocationService::find() and Criterion\Bookmark instead.
89+
*
8890
* {@inheritdoc}
8991
*/
9092
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)
99101
}
100102

101103
/**
104+
* @deprecated Please use LocationService::count() and Criterion\Bookmark instead.
105+
*
102106
* {@inheritdoc}
103107
*/
104108
public function countUserBookmarks(int $userId): int

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

+4
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ abstract public function loadBookmarkDataByUserIdAndLocationId(int $userId, arra
4444
/**
4545
* Load data for all bookmarks owned by given $userId.
4646
*
47+
* @deprecated Please use LocationService::find() and Criterion\Bookmark instead.
48+
*
4749
* @param int $userId ID of user
4850
* @param int $offset Offset to start listing from, 0 by default
4951
* @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
5557
/**
5658
* Count bookmarks owned by given $userId.
5759
*
60+
* @deprecated Please use LocationService::count() and Criterion\Bookmark instead.
61+
*
5862
* @param int $userId ID of user
5963
*
6064
* @return int

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

+4
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,8 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati
101101
}
102102

103103
/**
104+
* @deprecated Please use LocationService::find() and Criterion\Bookmark instead.
105+
*
104106
* {@inheritdoc}
105107
*/
106108
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)
123125
}
124126

125127
/**
128+
* @deprecated Please use LocationService::count() and Criterion\Bookmark instead.
129+
*
126130
* {@inheritdoc}
127131
*/
128132
public function countUserBookmarks(int $userId): int

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

+16
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,15 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati
5656
}
5757
}
5858

59+
/**
60+
* @deprecated Please use LocationService::find() and Criterion\Bookmark instead.
61+
*
62+
* @param int $userId
63+
* @param int $offset
64+
* @param int $limit
65+
*
66+
* @return array
67+
*/
5968
public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
6069
{
6170
try {
@@ -65,6 +74,13 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
6574
}
6675
}
6776

77+
/**
78+
* @deprecated Please use LocationService::count() and Criterion\Bookmark instead.
79+
*
80+
* @param int $userId
81+
*
82+
* @return int
83+
*/
6884
public function countUserBookmarks(int $userId): int
6985
{
7086
try {

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

+4
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ public function loadByUserIdAndLocationId(int $userId, array $locationIds): arra
7474
}
7575

7676
/**
77+
* @deprecated Please use LocationService::find() and Criterion\Bookmark instead.
78+
*
7779
* {@inheritdoc}
7880
*/
7981
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)
8486
}
8587

8688
/**
89+
* @deprecated Please use LocationService::count() and Criterion\Bookmark instead.
90+
*
8791
* {@inheritdoc}
8892
*/
8993
public function countUserBookmarks(int $userId): int

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

+47
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location;
10+
11+
use Doctrine\DBAL\ParameterType;
12+
use eZ\Publish\API\Repository\Values\Content\Query\Criterion\Bookmark;
13+
use eZ\Publish\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase;
14+
use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder;
15+
use eZ\Publish\SPI\Repository\Values\Filter\FilteringCriterion;
16+
17+
/**
18+
* @internal for internal use by Repository Filtering
19+
*/
20+
final class BookmarkQueryBuilder extends BaseLocationCriterionQueryBuilder
21+
{
22+
public function accepts(FilteringCriterion $criterion): bool
23+
{
24+
return $criterion instanceof Bookmark;
25+
}
26+
27+
public function buildQueryConstraint(
28+
FilteringQueryBuilder $queryBuilder,
29+
FilteringCriterion $criterion
30+
): ?string {
31+
$queryBuilder
32+
->joinOnce(
33+
'location',
34+
DoctrineDatabase::TABLE_BOOKMARKS,
35+
'bookmark',
36+
'location.node_id = bookmark.node_id'
37+
);
38+
39+
return $queryBuilder->expr()->eq(
40+
'bookmark.user_id',
41+
$queryBuilder->createNamedParameter(
42+
(int)$criterion->value[0],
43+
ParameterType::INTEGER
44+
)
45+
);
46+
}
47+
}

‎eZ/Publish/Core/Persistence/Legacy/Filter/SortClauseQueryBuilder/Bookmark/IdSortClauseQueryBuilder.php

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace eZ\Publish\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark;
10+
11+
use eZ\Publish\API\Repository\Values\Content\Query\SortClause\BookmarkId;
12+
use eZ\Publish\SPI\Persistence\Filter\Doctrine\FilteringQueryBuilder;
13+
use eZ\Publish\SPI\Repository\Values\Filter\FilteringSortClause;
14+
use eZ\Publish\SPI\Repository\Values\Filter\SortClauseQueryBuilder;
15+
16+
class IdSortClauseQueryBuilder implements SortClauseQueryBuilder
17+
{
18+
public function accepts(FilteringSortClause $sortClause): bool
19+
{
20+
return $sortClause instanceof BookmarkId;
21+
}
22+
23+
public function buildQuery(
24+
FilteringQueryBuilder $queryBuilder,
25+
FilteringSortClause $sortClause
26+
): void {
27+
/** @var \eZ\Publish\API\Repository\Values\Content\Query\SortClause $sortClause */
28+
$queryBuilder->addSelect('bookmark.id');
29+
$queryBuilder->addOrderBy('bookmark.id', $sortClause->direction);
30+
}
31+
}

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

+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
/**
4+
* @copyright Copyright (C) Ibexa AS. All rights reserved.
5+
* @license For full copyright and license information view LICENSE file distributed with this source code.
6+
*/
7+
declare(strict_types=1);
8+
9+
namespace eZ\Publish\Core\Persistence\Legacy\Tests\Filter\CriterionQueryBuilder\Location;
10+
11+
use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
12+
use eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder;
13+
use eZ\Publish\Core\Persistence\Legacy\Tests\Filter\BaseCriterionVisitorQueryBuilderTestCase;
14+
15+
/**
16+
* @covers \eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder::buildQueryConstraint
17+
* @covers \eZ\Publish\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location\BookmarkQueryBuilder::accepts
18+
*/
19+
final class BookmarkQueryBuilderTest extends BaseCriterionVisitorQueryBuilderTestCase
20+
{
21+
public function getFilteringCriteriaQueryData(): iterable
22+
{
23+
yield 'Bookmarks locations for user_id=14' => [
24+
new Criterion\Bookmark(14),
25+
'bookmark.user_id = :dcValue1',
26+
['dcValue1' => 14],
27+
];
28+
29+
yield 'Bookmarks locations for user_id=14 OR user_id=7' => [
30+
new Criterion\LogicalOr(
31+
[
32+
new Criterion\Bookmark(14),
33+
new Criterion\Bookmark(7),
34+
]
35+
),
36+
'(bookmark.user_id = :dcValue1) OR (bookmark.user_id = :dcValue2)',
37+
['dcValue1' => 14, 'dcValue2' => 7],
38+
];
39+
}
40+
41+
protected function getCriterionQueryBuilders(): iterable
42+
{
43+
return [new BookmarkQueryBuilder()];
44+
}
45+
}

‎eZ/Publish/Core/Repository/BookmarkService.php

+18-9
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,11 @@
1313
use eZ\Publish\API\Repository\Repository as RepositoryInterface;
1414
use eZ\Publish\API\Repository\Values\Bookmark\BookmarkList;
1515
use eZ\Publish\API\Repository\Values\Content\Location;
16+
use eZ\Publish\API\Repository\Values\Content\Query;
17+
use eZ\Publish\API\Repository\Values\Content\Query\Criterion;
18+
use eZ\Publish\API\Repository\Values\Content\Query\SortClause;
19+
use eZ\Publish\API\Repository\Values\Filter\Filter;
1620
use eZ\Publish\Core\Base\Exceptions\InvalidArgumentException;
17-
use eZ\Publish\SPI\Persistence\Bookmark\Bookmark;
1821
use eZ\Publish\SPI\Persistence\Bookmark\CreateStruct;
1922
use eZ\Publish\SPI\Persistence\Bookmark\Handler as BookmarkHandler;
2023

@@ -97,16 +100,22 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList
97100
{
98101
$currentUserId = $this->getCurrentUserId();
99102

100-
$list = new BookmarkList();
101-
$list->totalCount = $this->bookmarkHandler->countUserBookmarks($currentUserId);
102-
if ($list->totalCount > 0) {
103-
$bookmarks = $this->bookmarkHandler->loadUserBookmarks($currentUserId, $offset, $limit);
104-
105-
$list->items = array_map(function (Bookmark $bookmark) {
106-
return $this->repository->getLocationService()->loadLocation($bookmark->locationId);
107-
}, $bookmarks);
103+
$filter = new Filter();
104+
try {
105+
$filter
106+
->withCriterion(new Criterion\Bookmark($currentUserId))
107+
->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC))
108+
->sliceBy($limit, $offset);
109+
110+
$result = $this->repository->getlocationService()->find($filter, []);
111+
} catch (\eZ\Publish\API\Repository\Exceptions\BadStateException $e) {
112+
return new BookmarkList();
108113
}
109114

115+
$list = new BookmarkList();
116+
$list->totalCount = $result->totalCount;
117+
$list->items = $result->locations;
118+
110119
return $list;
111120
}
112121

0 commit comments

Comments
 (0)