IBX-5663: Added paginated role assignments load method to API

[barw4]

Question	Answer
JIRA issue	IBX-5663
Type	feature
Target Ibexa version	v3.3
BC breaks	no

2 new methods were added to RoleService in order to improve performance in the Back Office:

• loadRoleAssignments - allows setting $offset and $limit arguments in order to allow pagination. The old method getRoleAssignments even if cached could result in a huge performance drop in the Back Office as the amount of assignments can be really large.
• countRoleAssignments - counting role assignments, usable for the above method.

Related PR: ezsystems/ezplatform-admin-ui#2102

Checklist:

• Provided PR description.
• Tested the solution manually.
• Provided automated test coverage.
• Checked that target branch is set correctly (master for features, the oldest supported for bugs).
• Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
• Asked for a review (ping @ezsystems/engineering-team).

[ViniTou]

Is there a possibility to use proxies for policies instead of introducing a new method and later thinking why there are nulls for some cases?

[barw4]

@ViniTou I'll take a look into it.

[alongosz]

There are a business logic and security issues here. Currently there's no way to distinguish between a Role without policies from a Role without loaded policies. From API contract perspective information returned like that is not correct and could lead to treating the object as policy-less one, leading to security holes in places where we forgot or 3rd party is not aware that the information loaded is not full.

Is there a possibility to use proxies for policies instead of introducing a new method and later thinking why there are null for some cases?

Seems like the most reasonable way.

[barw4]

@alongosz @ViniTou as policies property in Role is an array and assuming we don't want to change that how would you want to approach building a proxy? ProxyGenerator requires an actual object to be proxied, so would you like to proxy every policy separately? It would require fetching ids of those policies either way so performance gain would be minimal.

[ViniTou]

@barw4
ArrayObject wouldnt probably be some solution, as I am almost sure it would fail at some level, and I think it will count as BC.

Other way, would be to introduce new class for that Role without policies, but probably above will apply as well.
Atm, no other ideas that wouldnt be some __get hacking things, but in current state I agree this is no go - it creates more problems than it resolves.

Is adding just pagination for role assigments not enough?

[barw4]

@ViniTou it should suffice for now, at least according to the public ticket. I've removed listRoles.

[alongosz]

Overall this is a good direction 💪
It needs a little bit of improvement:

[alongosz]

Note for reviewers: persistence cache for countRoleAssignments method was not implemented on purpose after a discovery phase and going back and forth on the solution.
It's because backoffice uses Trash and Content API to trash and then delete User Group, instead of User API. This makes role to user group assignment cache for count impossible to invalidate in a clean way.

We need to take another iteration to solve this separately, most likely after 4.6.0 LTS release.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
6 Code Smells

No Coverage information
0.0% Duplication

[micszo]

Reproduced the 30 seconds execution timeout with ca. 6k assigned users. Problem was occurring with new cache. Consecutive reloads took 7-8 seconds.
With the fix such role loads like any standard role, in 1-2 seconds in a dev env.

QA Approved on Ibexa Experience 3.3.34-dev with diffs.

[alongosz]

Nice work @barw4 💪