Skip to content

Conversation

@wiewiurdp
Copy link
Contributor

@wiewiurdp wiewiurdp commented Jan 5, 2026

🎫 Issue IBX-10186

Related PRs:

#600

Description:

This one is a community contribution from @ryanolee 🎉 .

This pull request introduces optional query limits to Repository filtering count operations and subtree size calculations in the Ibexa core codebase. The changes aim to improve performance and control resource consumption when performing counting and subtree operations on large datasets.

For QA:

Documentation:

@wiewiurdp wiewiurdp marked this pull request as ready for review January 7, 2026 10:59
@wiewiurdp wiewiurdp requested a review from a team January 7, 2026 10:59
Copy link
Contributor

@Steveb-p Steveb-p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$this->expectException(InvalidArgumentException::class);
$this->expectExceptionMessageMatches('/Limit must be greater than 0/');

$this->limitedCountQueryBuilder->wrap($qb, 'someField', 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add @phpstan-ignore argument.type since it's intentional break of contract :)

Copy link

@ryanolee ryanolee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good in terms of the original intention of restricting unbounded count queries to something much more processable by the CMS . Thanks for getting this into v5 core! 🎉

It should be noted that the companion PR to this might need to be brought up to date with v5 and is where much of the admin speedup from the changes made in this PR comes from ibexa/admin-ui#1605 given there are quite a few places in the CMS where these unbounded queries can result in significant slowdown!

Feel free to co-commit under rizza@rizza.net 🌟

@ryanolee
Copy link

ryanolee commented Jan 7, 2026

cc @Steveb-p @wiewiurdp 🎉

@konradoboza
Copy link
Contributor

Good point @ryanolee! @wiewiurdp could you please handle the companion PR accordingly?

* for a SiteAccess in a current context will be used.
*
* @param int|null $limit If set, the count will be limited to first $limit items found.
* In some cases it can significantly speed up a count operation for more complex filters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* In some cases it can significantly speed up a count operation for more complex filters.
* In some cases it can significantly speed up a count operation for more complex filters.

* If skipped, by default, unless SiteAccessAware layer has been disabled, languages set
* for a SiteAccess in a current context will be used.
* @param int|null $limit If set, the count will be limited to first $limit items found.
* In some cases it can significantly speed up a count operation for more complex filters.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* In some cases it can significantly speed up a count operation for more complex filters.
* In some cases it can significantly speed up a count operation for more complex filters.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jan 7, 2026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants