Fix performance problem when searching for groups in the groupfolder app

[CarlSchwan]

The scope of this PR ended up a bit bigger than initially thought.

• Add a method in group backends to search user for userId/display name and directly return User object (lazy with display name possibly preloaded). This should be nicer in terms of performance as we don't need to separately load the display names (e.g. when searching in the ui)
• Add a lot of type hinting + doc in IGroup/Group
• Remove the unused Backend since it was unused
• Fix various psalm issues and avoid calling the deprecated method implementsActions

This target NC 25, but we might do a partial backport to NC 24 and earlier (the optimization)

[blizzz]

All nice, with the one downside that the search now happens on terms of the group backend. Group backends able to contain users of mixed user backends thus do not utilize the search methods of the user backends. For example, when normally you can find "Jeanne Doe" of LDAP by her employee number 123456, you cannot achieve the same when search for her in the local group. The sad thing of course is that the owning user backend does not know about this relationship, so searching there will would also be painfully slow (likely better than now, but still).

With that in mind, this approach may be acceptable with this known limitation (that should be documented somewhere). Additional the slower search can be offered still and enabled via config option – and as opt-in. Searching for display name* and email is the default case after all.

*also there might be different results

[CarlSchwan]

All nice, with the one downside that the search now happens on terms of the group backend. Group backends able to contain users of mixed user backends thus do not utilize the search methods of the user backends. For example, when normally you can find "Jeanne Doe" of LDAP by her employee number 123456, you cannot achieve the same when search for her in the local group. The sad thing of course is that the owning user backend does not know about this relationship, so searching there will would also be painfully slow (likely better than now, but still).

According to the code, the search happens like this with this patch:

• OC\Group\Manager::displayNamesInGroup
• OC\Group\Group::searchUsers - Call searchInGroup for each group backend
  • Database backend: will look for any displayname+userId with the corresponding gid
  • LDAP backend: merge the results of getUsersInPrimaryGroup, getUsersInGidNumber and _groupMembers (recursive)

Before:

• OC\Group\Manager::displayNamesInGroup
• OC\User\UserManager::searchDisplayName with an high limit
  • Database backend: will look for any displayname+userId discarding the gid information
  • LDAP backend: call getUsers that then do an LDAP query with getFilterPartForUserSearch
• In the resulting results, look if they are part of the group
  • Database backend: a db query for each user matching the search filtr
  • LDAP backend: search if the user is in the primary group and if not do __groupMembers (recursive) and check if the user is found in the group members returned

So to me, this seems equivalent just faster and cleaner, unless I obliviously missed something. Maybe we should do a call on Monday to discuss this if you have time?

[CarlSchwan]

ping :)

[CarlSchwan]

friendly ping :)

[PVince81]

/rebase

[CarlSchwan]

@blizzz I reverted the changes for the search and found out instead other areas where we could improve the perf by quite a bit:

• When searching with the database group backend, cache the displayname
• Implement a batch groupExists and getGroupDetails methods (with a fallback when not implemented).

In total, with 1100 LDAP groups, these changes reduce the SQL query count from 1125 to 13. The LDAP query count is still very high (>6000) but I have a PR in groupfolder to reduce it to 30.

I'll try to split the change into multiple self-contained PRs to make it easier to review :)

[CarlSchwan]

First split #32866

[skjnldsv]

As there is no feedback since a while I will close this ticket.
If you will decide to work on this feature again and if it hasn't been fixed or implemented already, feel free to re-open and solve the various conflicts.