Skip to content
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

Limit ldap user count #50171

Merged
merged 6 commits into from
Jan 16, 2025
Merged

Limit ldap user count #50171

merged 6 commits into from
Jan 16, 2025

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Jan 14, 2025

  • Resolves: #

Summary

This avoids listing all LDAP users only to check if there are more than 100 for the updatenotification application. This speeds up opening admin settings a lot on big instances with LDAP backend.
The new user count limit is made so that it can be implemented in other backends if needed, and places calling the user count only to compare against a threshold have been adapted to set a limit.

Checklist

@come-nc come-nc added the 3. to review Waiting for reviews label Jan 14, 2025
@come-nc come-nc self-assigned this Jan 14, 2025
@come-nc come-nc added this to the Nextcloud 31 milestone Jan 14, 2025
@skjnldsv skjnldsv mentioned this pull request Jan 14, 2025
@come-nc come-nc requested review from nickvergessen, blizzz, a team, icewind1991, Altahrim and nfebe and removed request for a team January 14, 2025 16:35
@come-nc
Copy link
Contributor Author

come-nc commented Jan 14, 2025

cypress failure is unrelated

Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

few remarks

lib/private/User/Manager.php Show resolved Hide resolved
lib/private/User/Database.php Show resolved Hide resolved
apps/user_ldap/lib/User_LDAP.php Show resolved Hide resolved
@come-nc come-nc added the pending documentation This pull request needs an associated documentation update label Jan 16, 2025
@come-nc come-nc merged commit 626bc72 into master Jan 16, 2025
188 checks passed
@come-nc come-nc deleted the enh/limit-ldap-user-count branch January 16, 2025 16:25
@come-nc
Copy link
Contributor Author

come-nc commented Jan 16, 2025

Documented in nextcloud/documentation#12513

@come-nc come-nc removed the pending documentation This pull request needs an associated documentation update label Jan 16, 2025
@skjnldsv skjnldsv mentioned this pull request Jan 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants