perf: resolve group membership once per folder listing instead of once per entry (#28810)

Listing a user's folders re-checks which entries they may still see, and it resolved their group membership again for every folder, then again inside the collection and note branches for every entry. A comment in that helper claims one membership fetch for the whole listing, but the caller invokes it once per folder, so the claim never held.

The listing now resolves membership once, and only when some folder actually carries entries, then threads it through the file, collection and note checks. Callers that do not supply it are unchanged and still resolve for themselves.

Measured with twenty folders holding six files, two knowledge bases and two notes each: 245 queries and ~145 ms before, 186 and ~117 ms after. The folders returned, and the entries the integrity pass writes back, are unchanged. That was checked against entries the caller owns, entries shared through a group, entries shared with nobody, another user's files, and an unrecognised entry type.
This commit is contained in:
Classic298
2026-08-19 11:16:29 -07:00
committed by GitHub
parent 767a1157f1
commit 7cf6051a74
3 changed files with 25 additions and 7 deletions
@@ -128,6 +128,7 @@ async def get_accessible_folder_files(
entries: list[dict] | None,
user: UserModel,
db: AsyncSession | None = None,
user_group_ids: set[str] | None = None,
) -> list[dict]:
"""Filter folder.data['files'] entries to those the caller can read.
@@ -144,8 +145,8 @@ async def get_accessible_folder_files(
if user.role == 'admin':
return entries
# One group-membership fetch for the whole folder listing
user_group_ids = {group.id for group in await Groups.get_groups_by_member_id(user.id, db=db)}
if user_group_ids is None:
user_group_ids = {group.id for group in await Groups.get_groups_by_member_id(user.id, db=db)}
accessible: list[dict] = []
for entry in entries:
@@ -155,7 +156,9 @@ async def get_accessible_folder_files(
if await has_access_to_file(entry_id, 'read', user, db=db, user_group_ids=user_group_ids):
accessible.append(entry)
elif entry_type == 'collection':
if await Knowledges.check_access_by_user_id(entry_id, user.id, 'read', db=db):
if await Knowledges.check_access_by_user_id(
entry_id, user.id, 'read', db=db, user_group_ids=user_group_ids
):
accessible.append(entry)
elif entry_type == 'note':
# Owner has no self-grant (notes are private by default), so check ownership too.
@@ -169,6 +172,7 @@ async def get_accessible_folder_files(
resource_type='note',
resource_id=entry_id,
permission='read',
user_group_ids=user_group_ids,
db=db,
)
):