-
Notifications
You must be signed in to change notification settings - Fork 17
IBX-10186: Added limits to Repository Filtering count and subtree queries #600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 4.6
Are you sure you want to change the base?
Changes from all commits
00e41bc
2283e73
e0112db
88043e8
d886f29
3beb674
fa1ee68
5b2fe00
804f1a7
12fcae7
2720425
3674837
7d55dd0
1f33f28
162806f
8c99daf
9d2d107
80fe453
f25060d
eb83b30
500b400
2c1b555
a54d922
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, as a consequence of what @konradoboza mentioned earlier, we don't need to introduce here an extra interface. Explanation: Repository services do not undergo SPI [1] Backwards compatibility promise. This means that we only promise BC on consumption. In the past, throughout the life-cycle of eZ Platform 1.x, 2.x, Ibexa 3.x, 4.x, we've added methods and optional arguments to methods on both API and persistence level. Persistence layer is even explicitly documented as not frozen in README.md. The reason is maintenance and stability - technical issues (like the one you've experienced with a proxy). We support extending only classes and interfaces which have been explicitly documented in the Ibexa Developer Documentation for that purpose, e.g., field type classes. Doing this in any other way would make very hard to add new features to the Product. [1] In this context - a Service Provider Interface - class/interface meant for extending. Treating everything as SPI seems to be a default BC policy for Symfony. We approach this a bit differently. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| <?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\Future\Repository; | ||
|
|
||
| use Ibexa\Contracts\Core\Repository\ContentService; | ||
| use Ibexa\Contracts\Core\Repository\Values\Filter\Filter; | ||
|
|
||
| /** | ||
| * @internal Future version of ContentService to be released in Ibexa Core 5.0. Used to force value proxies to generate correct types. For internal use only. | ||
| */ | ||
| interface FutureContentService extends ContentService | ||
| { | ||
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int; | ||
| } |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same comment here. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| <?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\Future\Repository; | ||
|
|
||
| use Ibexa\Contracts\Core\Repository\LocationService; | ||
| use Ibexa\Contracts\Core\Repository\Values\Content\Location; | ||
| use Ibexa\Contracts\Core\Repository\Values\Filter\Filter; | ||
|
|
||
| /** | ||
| * @internal Future version of LocationService to be released in Ibexa Core 5.0. Used to force value proxies to generate correct types. For internal use only. | ||
| */ | ||
| interface FutureLocationService extends LocationService | ||
| { | ||
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| public function getLocationChildCount(Location $location, ?int $limit = null): int; | ||
|
|
||
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| public function getSubtreeSize(Location $location, ?int $limit = null): int; | ||
|
|
||
| /** | ||
| * {@inheritDoc} | ||
| */ | ||
| public function count(Filter $filter, ?array $languages = null, ?int $limit = null): int; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -110,7 +110,11 @@ public function loadParentLocationsForDraftContent($contentId); | |
| */ | ||
| public function copySubtree($sourceId, $destinationParentId); | ||
|
|
||
| public function getSubtreeSize(string $path): int; | ||
| /** | ||
| * @param int|null $limit | ||
| */ | ||
| // @phpstan-ignore parameter.notFound | ||
| public function getSubtreeSize(string $path /* ?int $limit = null */): int; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here you can simply add that parameter to the method as well. Additionally, you don't need to add extra PHPDoc block if it doesn't provide any extra information. |
||
|
|
||
| /** | ||
| * Moves location identified by $sourceId into new parent identified by $destinationParentId. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expectation remark: after addressing the feedback we expect here not to have any new issues, only removals.
If something unrelated to your changes pops up after rebasing onto main, please let me know. We address such issues regularly as they occur quite often.