Use existence queries for org group membership checks - #261
Merged
Conversation
Org.get_user_org_group previously used 'user in queryset' checks which load all members of each role; with large orgs this fetched thousands of user rows on every request via the context processor and permission checks. Replace with .filter(id=...).exists() lookups and add a query count test.
Cache the computed org group on the user object so repeated calls within a request (context processor plus permission checks) only query once. Replace the query count test with assertions on the SQL shape of the membership checks, since counting queries alone did not distinguish existence queries from fetching full member lists. Also cover the anonymous user case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Org.get_user_org_groupchecked membership withuser in queryset, which loads the entire administrators/editors/viewers member lists — and it runs on every request via the context processor and permission checks, so large orgs loaded thousands of User rows per request.Membership is now checked with
.filter(id=user.id).exists(), with identical return values, and the computed group is cached on the user object (mirroring the existing_orgpattern) so the repeated per-request calls only pay for the lookup once.The test asserts the SQL shape of the membership checks — each must be an existence query (
SELECT 1 AS ...) that doesn't hydrate user columns — which fails against the old implementation, unlike a plain query count. It also covers the anonymous user case and verifies cached lookups issue no queries.