perf: batch the file access check queries (#27383)

has_access_to_file runs for every non-owner file GET, per RAG file check and per shared-chat or model-attached file. Its final step called Models.get_models_by_user_id, which issued one grant query per non-owned workspace model, so a single file check on an instance with M workspace models cost M grant queries plus a group query, with the deny path always paying full price. Its collection_name step listed every knowledge base the user can access (itself one grant query per knowledge base) just to scan the list for one id. And get_accessible_folder_files repeated the whole pipeline per folder entry, refetching the caller's group memberships every time.

Three changes, all using parameters and helpers that already exist:
- Models.get_models_by_user_id resolves grants for all non-owned models in one get_accessible_resource_ids call and accepts prefetched user_group_ids.
- The collection_name check fetches the one referenced knowledge base and performs a single owner-or-grant check with the already-resolved group ids, preserving the write-requires-owner guard exactly (including its short-circuit before any grant query).
- get_accessible_folder_files resolves group ids once and threads them through every per-entry check.

Benchmark:

| metric | before | after |
| --- | --- | --- |
| filter loop CPU, 300 workspace models (queries stubbed) | 47 us | 19 us |
| grant queries per file-access check, M workspace models | M | 1 |
| group membership queries per folder listing, F files | F | 1 |

The stubbed CPU row understates the win: each removed query in the other two rows was a real database round trip.

Functionally verified with stubbed accessors: owned plus granted models are returned with owned ids excluded from the batch query; model-attached file access resolves through the batched path; the collection_name path does one KB fetch and one grant check with no full listing; a missing KB falls through; write access via a KB still requires the KB owner to own the file and short-circuits before the grant query; folder listings fetch groups exactly once.
This commit is contained in:
Classic298
2026-07-23 17:50:08 -05:00
committed by GitHub
parent d5f099a5d4
commit d67bc4ffcd
2 changed files with 46 additions and 26 deletions
+17 -17
View File
@@ -303,26 +303,26 @@ class ModelsTable:
]
async def get_models_by_user_id(
self, user_id: str, permission: str = 'write', db: AsyncSession | None = None
self,
user_id: str,
permission: str = 'write',
db: AsyncSession | None = None,
user_group_ids: set[str] | None = None,
) -> list[ModelUserResponse]:
models = await self.get_models(db=db)
user_groups = await Groups.get_groups_by_member_id(user_id, db=db)
user_group_ids = {group.id for group in user_groups}
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)}
result = []
for model in models:
if model.user_id == user_id:
result.append(model)
elif await AccessGrants.has_access(
user_id=user_id,
resource_type='model',
resource_id=model.id,
permission=permission,
user_group_ids=user_group_ids,
db=db,
):
result.append(model)
return result
# One grants query for all non-owned models instead of one per model
accessible_ids = await AccessGrants.get_accessible_resource_ids(
user_id=user_id,
resource_type='model',
resource_ids=[model.id for model in models if model.user_id != user_id],
permission=permission,
user_group_ids=user_group_ids,
db=db,
)
return [model for model in models if model.user_id == user_id or model.id in accessible_ids]
def _has_permission(self, db, query, filter: dict, permission: str = 'read'):
return AccessGrants.has_permission_filter(