Skip to content

Commit 9164daa

Browse files
vidarlvidarl
committed
IBX-6773: Bookmarks for non-accessible contents cause exception
1 parent 2af1859 commit 9164daa

File tree

16 files changed

+267
-62
lines changed

16 files changed

+267
-62
lines changed

‎phpstan-baseline.neon

+5-5
Original file line numberDiff line numberDiff line change
@@ -18165,6 +18165,11 @@ parameters:
1816518165
count: 1
1816618166
path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/AncestorQueryBuilder.php
1816718167

18168+
-
18169+
message: "#^Cannot access offset 0 on array\\<bool\\|float\\|int\\|string\\>\\|bool\\|float\\|int\\|string\\.$#"
18170+
count: 1
18171+
path: src/lib/Persistence/Legacy/Filter/CriterionQueryBuilder/Location/BookmarkQueryBuilder.php
18172+
1816818173
-
1816918174
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\\.$#"
1817018175
count: 1
@@ -57575,11 +57580,6 @@ parameters:
5757557580
count: 1
5757657581
path: tests/lib/Repository/Service/Mock/BookmarkTest.php
5757757582

57578-
-
57579-
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\Repository\\\\Service\\\\Mock\\\\BookmarkTest\\:\\:testLoadBookmarksEmptyList\\(\\) has no return type specified\\.$#"
57580-
count: 1
57581-
path: tests/lib/Repository/Service/Mock/BookmarkTest.php
57582-
5758357583
-
5758457584
message: "#^Method Ibexa\\\\Tests\\\\Core\\\\Repository\\\\Service\\\\Mock\\\\BookmarkTest\\:\\:testLocationShouldBeBookmarked\\(\\) has no return type specified\\.$#"
5758557585
count: 1

‎src/contracts/Persistence/Bookmark/Handler.php

+4
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@ public function loadUserIdsByLocation(Location $location): array;
5050
/**
5151
* Loads bookmarks owned by user.
5252
*
53+
* @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
54+
*
5355
* @param int $userId
5456
* @param int $offset the start offset for paging
5557
* @param int $limit the number of bookmarked locations returned
@@ -61,6 +63,8 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
6163
/**
6264
* Count bookmarks owned by user.
6365
*
66+
* @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
67+
*
6468
* @param int $userId
6569
*
6670
* @return int

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

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
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 Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
10+
11+
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
12+
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\Operator\Specifications;
13+
use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion;
14+
15+
/**
16+
* A criterion that matches locations of bookmarks for a given user id.
17+
*
18+
* Supported operators:
19+
* - EQ: matches against a unique user id
20+
*/
21+
final class IsBookmarked extends Criterion implements FilteringCriterion
22+
{
23+
/**
24+
* Creates a new IsBookmarked criterion.
25+
*
26+
* @param int $value user id for which bookmarked locations must be matched against
27+
*
28+
* @throws \InvalidArgumentException if a non numeric id is given
29+
* @throws \InvalidArgumentException if the value type doesn't match the operator
30+
*/
31+
public function __construct($value)
32+
{
33+
parent::__construct(null, null, $value);
34+
}
35+
36+
public function getSpecifications(): array
37+
{
38+
return [
39+
new Specifications(Operator::EQ, Specifications::FORMAT_SINGLE, Specifications::TYPE_INTEGER),
40+
];
41+
}
42+
}

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

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
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 Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause;
10+
11+
use Ibexa\Contracts\Core\Repository\Values\Content\Query;
12+
use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause;
13+
use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause;
14+
15+
/**
16+
* Sets sort direction on the bookmark id for a location query containing IsBookmarked criterion.
17+
*/
18+
final class BookmarkId extends SortClause implements FilteringSortClause
19+
{
20+
public function __construct(string $sortDirection = Query::SORT_ASC)
21+
{
22+
parent::__construct('id', $sortDirection);
23+
}
24+
}

‎src/lib/Persistence/Cache/BookmarkHandler.php

+4
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ function (Bookmark $bookmark) {
8383
}
8484

8585
/**
86+
* @deprecated The "BookmarkHandler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
87+
*
8688
* {@inheritdoc}
8789
*/
8890
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)
9799
}
98100

99101
/**
102+
* @deprecated The "BookmarkHandler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
103+
*
100104
* {@inheritdoc}
101105
*/
102106
public function countUserBookmarks(int $userId): int

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

+4
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ abstract public function loadUserIdsByLocation(Location $location): array;
5252
/**
5353
* Load data for all bookmarks owned by given $userId.
5454
*
55+
* @deprecated Gateway::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
56+
*
5557
* @param int $userId ID of user
5658
* @param int $offset Offset to start listing from, 0 by default
5759
* @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
6365
/**
6466
* Count bookmarks owned by given $userId.
6567
*
68+
* @deprecated The "Gateway::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
69+
*
6670
* @param int $userId ID of user
6771
*
6872
* @return int

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

+4
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ public function loadUserIdsByLocation(Location $location): array
124124
}
125125

126126
/**
127+
* @deprecated The "DoctrineDatabase::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
128+
*
127129
* {@inheritdoc}
128130
*/
129131
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)
146148
}
147149

148150
/**
151+
* @deprecated The "DoctrineDatabase::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
152+
*
149153
* {@inheritdoc}
150154
*/
151155
public function countUserBookmarks(int $userId): int

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

+8
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ public function loadBookmarkDataByUserIdAndLocationId(int $userId, array $locati
5757
}
5858
}
5959

60+
/**
61+
* @deprecated The "ExceptionConversion::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
62+
*
63+
* @return array
64+
*/
6065
public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1): array
6166
{
6267
try {
@@ -66,6 +71,9 @@ public function loadUserBookmarks(int $userId, int $offset = 0, int $limit = -1)
6671
}
6772
}
6873

74+
/**
75+
* @deprecated The "ExceptionConversion::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
76+
*/
6977
public function countUserBookmarks(int $userId): int
7078
{
7179
try {

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

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

7777
/**
78+
* @deprecated The "Handler::loadUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::find()" and "Criterion\IsBookmarked" instead.
79+
*
7880
* {@inheritdoc}
7981
*/
8082
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)
8587
}
8688

8789
/**
90+
* @deprecated The "Handler::countUserBookmarks()" method is deprecated, will be removed in 5.0.0. Use "LocationService::count()" and "Criterion\IsBookmarked" instead.
91+
*
8892
* {@inheritdoc}
8993
*/
9094
public function countUserBookmarks(int $userId): int

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

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
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 Ibexa\Core\Persistence\Legacy\Filter\CriterionQueryBuilder\Location;
10+
11+
use Doctrine\DBAL\ParameterType;
12+
use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
13+
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked;
14+
use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringCriterion;
15+
use Ibexa\Core\Persistence\Legacy\Bookmark\Gateway\DoctrineDatabase;
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 IsBookmarked;
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+
/** @var \Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion\IsBookmarked $criterion */
40+
return $queryBuilder->expr()->eq(
41+
'bookmark.user_id',
42+
$queryBuilder->createNamedParameter(
43+
(int)$criterion->value[0],
44+
ParameterType::INTEGER
45+
)
46+
);
47+
}
48+
}

‎src/lib/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 Ibexa\Core\Persistence\Legacy\Filter\SortClauseQueryBuilder\Bookmark;
10+
11+
use Ibexa\Contracts\Core\Persistence\Filter\Doctrine\FilteringQueryBuilder;
12+
use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause\BookmarkId;
13+
use Ibexa\Contracts\Core\Repository\Values\Filter\FilteringSortClause;
14+
use Ibexa\Contracts\Core\Repository\Values\Filter\SortClauseQueryBuilder;
15+
16+
final 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 \Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause $sortClause */
28+
$queryBuilder->addSelect('bookmark.id');
29+
$queryBuilder->addOrderBy('bookmark.id', $sortClause->direction);
30+
}
31+
}

‎src/lib/Repository/BookmarkService.php

+30-10
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,20 @@
99
namespace Ibexa\Core\Repository;
1010

1111
use Exception;
12-
use Ibexa\Contracts\Core\Persistence\Bookmark\Bookmark;
1312
use Ibexa\Contracts\Core\Persistence\Bookmark\CreateStruct;
1413
use Ibexa\Contracts\Core\Persistence\Bookmark\Handler as BookmarkHandler;
1514
use Ibexa\Contracts\Core\Repository\BookmarkService as BookmarkServiceInterface;
15+
use Ibexa\Contracts\Core\Repository\Exceptions\BadStateException;
1616
use Ibexa\Contracts\Core\Repository\Repository as RepositoryInterface;
1717
use Ibexa\Contracts\Core\Repository\Values\Bookmark\BookmarkList;
1818
use Ibexa\Contracts\Core\Repository\Values\Content\Location;
19+
use Ibexa\Contracts\Core\Repository\Values\Content\Query;
20+
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
21+
use Ibexa\Contracts\Core\Repository\Values\Content\Query\SortClause;
22+
use Ibexa\Contracts\Core\Repository\Values\Filter\Filter;
1923
use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
24+
use Psr\Log\LoggerInterface;
25+
use Psr\Log\NullLogger;
2026

2127
class BookmarkService implements BookmarkServiceInterface
2228
{
@@ -26,16 +32,20 @@ class BookmarkService implements BookmarkServiceInterface
2632
/** @var \Ibexa\Contracts\Core\Persistence\Bookmark\Handler */
2733
protected $bookmarkHandler;
2834

35+
/** @var \Psr\Log\LoggerInterface */
36+
private $logger;
37+
2938
/**
3039
* BookmarkService constructor.
3140
*
3241
* @param \Ibexa\Contracts\Core\Repository\Repository $repository
3342
* @param \Ibexa\Contracts\Core\Persistence\Bookmark\Handler $bookmarkHandler
3443
*/
35-
public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler)
44+
public function __construct(RepositoryInterface $repository, BookmarkHandler $bookmarkHandler, LoggerInterface $logger = null)
3645
{
3746
$this->repository = $repository;
3847
$this->bookmarkHandler = $bookmarkHandler;
48+
$this->logger = $logger ?? new NullLogger();
3949
}
4050

4151
/**
@@ -97,16 +107,26 @@ public function loadBookmarks(int $offset = 0, int $limit = 25): BookmarkList
97107
{
98108
$currentUserId = $this->getCurrentUserId();
99109

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);
110+
$filter = new Filter();
111+
try {
112+
$filter
113+
->withCriterion(new Criterion\IsBookmarked($currentUserId))
114+
->withSortClause(new SortClause\BookmarkId(Query::SORT_DESC))
115+
->sliceBy($limit, $offset);
116+
117+
$result = $this->repository->getlocationService()->find($filter, []);
118+
} catch (BadStateException $e) {
119+
$this->logger->debug($e->getMessage(), [
120+
'exception' => $e,
121+
]);
122+
123+
return new BookmarkList();
108124
}
109125

126+
$list = new BookmarkList();
127+
$list->totalCount = $result->totalCount;
128+
$list->items = $result->locations;
129+
110130
return $list;
111131
}
112132

‎src/lib/Repository/Repository.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,8 @@ public function getBookmarkService(): BookmarkServiceInterface
602602
if ($this->bookmarkService === null) {
603603
$this->bookmarkService = new BookmarkService(
604604
$this,
605-
$this->persistenceHandler->bookmarkHandler()
605+
$this->persistenceHandler->bookmarkHandler(),
606+
$this->logger
606607
);
607608
}
608609

0 commit comments

Comments
 (0)