simplify and speed up search in groups #32197
Conversation
Searching users in groups is actually implemented and should be reused Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
blizzz
added
bug
2. developing
lib/private/Group/Manager.php
Outdated
| $groupUsers = $group->searchUsers('', $limit, $offset); | ||
| } | ||
|
|
||
| $users = $group->searchUsers(trim($search), $limit === -1 ? null : $limit, $offset); |
There was a problem hiding this comment.
I had the same idea but this is a search by userid and not disolay name. At least for the database backend, i have some changes locally to add a search by display name too, but I have not finished it.
Also probably a good idea to use the new DisplayNameCache
There was a problem hiding this comment.
I had the same idea but this is a search by userid and not disolay name. At least for the database backend, i have some changes locally to add a search by display name too, but I have not finished it.
This explains it m(
Also probably a good idea to use the new DisplayNameCache
yeah
There was a problem hiding this comment.
IGroup::searchDisplayName is what we should use. But the implementation is the same as searchUsers.
There was a problem hiding this comment.
Yep, I push my work tomorrow morning 👍
There was a problem hiding this comment.
@CarlSchwan is this what you are doing^?
LDAP backend does a proper search in any case. Normally a search for user IDs does not happen and does not have value in itself 🤔
There was a problem hiding this comment.
It's trickling in slowly 🧠. The group DB backend may not only check DB users, because it is decoupled. You can add LDAP users into local groups.
There was a problem hiding this comment.
Maybe cycle through the users and extend IUser with satisfy(string $search): bool. Downside: will still have a request per users against DB, LDAP or whichever source. Better extend user backend with a batch operations filterUsers(array $userIds, string $search): array.
There was a problem hiding this comment.
It's trickling in slowly brain. The group DB backend may not only check DB users, because it is decoupled. You can add LDAP users into local groups.
$group->searchDisplayName will do a search for each group backend so it should still work if you have a ldap user inside a db group. I need to do a bit more testing ;)
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
|
@blizzz ant update? wanna rebase? :) |
🧹 💨 🤭 gosh, the dust! I am not aware what the state was with @CarlSchwan's changes; and also not having time currently. Pending between keeping it open indefinitely or closing. |
|
Closing this pull request due to lack of recent activity and updates. We appreciate your contribution and encourage you to reopen or provide further updates if necessary. |
Searching users in groups is actually implemented and should be reused
(didn't test myself yet :) )
To fix #32188