perf(grants,favorites): batch resource resolution to kill N+1 and pool fan-out
Three list endpoints resolved each resource with one query per id: - GET /api/grants/incoming and /api/grants/outgoing used join_all(ids.map(get_file)) + join_all(ids.map(get_folder)), so a single page (limit ≤ 200) could demand ~200 concurrent connections from the 20-connection primary pool, causing acquire-timeouts and head-of-line blocking under load. - The NextCloud favorites REPORT (oc:filter-files) fetched get_file/ get_folder once per favorite — up to N serial round-trips per sync. Add by-ids batch reads that mirror the existing get_file/get_folder column mapping and NOT is_trashed filter: - FileBlobReadRepository::get_files_by_ids / FolderDbRepository::get_folders_by_ids (one SELECT ... WHERE id = ANY($1)), exposed as FileRetrievalService:: get_files_by_ids / FolderService::get_folders_by_ids returning DTOs. - Both grant handlers and the favorites REPORT now issue two batch queries total and look results up by id, preserving original order. Missing ids (stale grants whose resource was deleted, or trashed/removed favorites) drop out exactly as before. No auth-semantics change: these paths already resolved ids vetted by the authorization engine / favorites table. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TAzLEQDaLak3dnrEN3YT35
This commit is contained in:
@@ -250,6 +250,17 @@ impl FileRetrievalService {
|
||||
let stream = self.file_read.get_file_stream(id).await?;
|
||||
Ok((dto, OptimizedFileContent::Stream(Box::into_pin(stream))))
|
||||
}
|
||||
|
||||
/// Batch counterpart of [`FileRetrievalUseCase::get_file`]: resolve many
|
||||
/// file ids in ONE query instead of one per id. Like `get_file` it
|
||||
/// performs no per-file authorization — both current callers (ACL grant
|
||||
/// listing, NextCloud favorites REPORT) resolve ids already vetted by the
|
||||
/// authorization engine or the favorites table. Missing or trashed ids are
|
||||
/// absent from the result; callers re-associate by `id`.
|
||||
pub async fn get_files_by_ids(&self, ids: &[String]) -> Result<Vec<FileDto>, DomainError> {
|
||||
let files = self.file_read.get_files_by_ids(ids).await?;
|
||||
Ok(files.into_iter().map(FileDto::from).collect())
|
||||
}
|
||||
}
|
||||
|
||||
impl FileRetrievalUseCase for FileRetrievalService {
|
||||
|
||||
@@ -29,6 +29,17 @@ impl FolderService {
|
||||
}
|
||||
}
|
||||
|
||||
/// Batch counterpart of `get_folder`: resolve many folder ids in ONE
|
||||
/// query instead of one per id. Like `get_folder` it performs no
|
||||
/// per-folder authorization — both current callers (ACL grant listing,
|
||||
/// NextCloud favorites REPORT) resolve ids already vetted by the
|
||||
/// authorization engine or the favorites table. Missing or trashed ids
|
||||
/// are absent from the result; callers re-associate by `id`.
|
||||
pub async fn get_folders_by_ids(&self, ids: &[String]) -> Result<Vec<FolderDto>, DomainError> {
|
||||
let folders = self.folder_storage.get_folders_by_ids(ids).await?;
|
||||
Ok(folders.into_iter().map(FolderDto::from).collect())
|
||||
}
|
||||
|
||||
/// Helper: parse a folder id string into a `Resource::Folder`. Returns
|
||||
/// `DomainError::not_found` on parse error (anti-enumeration — the same
|
||||
/// error as "folder does not exist").
|
||||
|
||||
@@ -255,6 +255,52 @@ impl FileBlobReadRepository {
|
||||
})
|
||||
}
|
||||
|
||||
/// Batch-fetch files by id — the by-ids counterpart of [`get_file`],
|
||||
/// used to resolve a page of ACL grants or favorites in ONE round-trip
|
||||
/// instead of one query per id (the previous `join_all(ids.map(get_file))`
|
||||
/// could fan out to ~200 concurrent pooled connections per page). Applies
|
||||
/// the same `NOT is_trashed` filter and identical column mapping as
|
||||
/// `get_file`. Ids that are missing or trashed simply drop out, so callers
|
||||
/// must re-associate results by id; ordering is not guaranteed.
|
||||
pub async fn get_files_by_ids(&self, ids: &[String]) -> Result<Vec<File>, DomainError> {
|
||||
let uuid_ids: Vec<Uuid> = ids.iter().filter_map(|id| id.parse().ok()).collect();
|
||||
if uuid_ids.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
let rows = sqlx::query_as::<_, FileRow>(
|
||||
"SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, \
|
||||
fi.size, fi.mime_type, \
|
||||
EXTRACT(EPOCH FROM fi.created_at)::bigint, \
|
||||
EXTRACT(EPOCH FROM fi.updated_at)::bigint, \
|
||||
fi.blob_hash, \
|
||||
fi.user_id \
|
||||
FROM storage.files fi \
|
||||
LEFT JOIN storage.folders fo ON fo.id = fi.folder_id \
|
||||
WHERE fi.id = ANY($1) AND NOT fi.is_trashed",
|
||||
)
|
||||
.bind(&uuid_ids)
|
||||
.fetch_all(self.pool.as_ref())
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("FileBlobRead", format!("get_files_by_ids: {e}"))
|
||||
})?;
|
||||
|
||||
rows.into_iter()
|
||||
.map(
|
||||
|(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid)| {
|
||||
Self::row_to_file(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid)
|
||||
},
|
||||
)
|
||||
.collect::<Result<Vec<_>, _>>()
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error(
|
||||
"FileBlobRead",
|
||||
format!("get_files_by_ids mapping: {e}"),
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
/// Returns the user_id (owner) for a given file ID.
|
||||
/// Mirrors `FolderDbRepository::get_folder_user_id`.
|
||||
/// Used by the AuthorizationEngine for owner short-circuit.
|
||||
|
||||
@@ -109,6 +109,37 @@ impl FolderDbRepository {
|
||||
)
|
||||
.map_err(|e| DomainError::internal_error("FolderDb", format!("entity: {e}")))
|
||||
}
|
||||
|
||||
/// Batch-fetch folders by id — the by-ids counterpart of `get_folder`,
|
||||
/// resolving a page of ACL grants or favorites in ONE query instead of
|
||||
/// one per id. Same `NOT is_trashed` filter and column mapping as
|
||||
/// `get_folder`; missing or trashed ids drop out and callers re-associate
|
||||
/// by id; ordering is not guaranteed.
|
||||
pub async fn get_folders_by_ids(&self, ids: &[String]) -> Result<Vec<Folder>, DomainError> {
|
||||
let uuid_ids: Vec<Uuid> = ids.iter().filter_map(|id| id.parse().ok()).collect();
|
||||
if uuid_ids.is_empty() {
|
||||
return Ok(Vec::new());
|
||||
}
|
||||
|
||||
let rows = sqlx::query_as::<_, FolderRow>(
|
||||
r#"
|
||||
SELECT id::text, name, path, parent_id::text, user_id,
|
||||
EXTRACT(EPOCH FROM created_at)::bigint,
|
||||
EXTRACT(EPOCH FROM updated_at)::bigint,
|
||||
EXTRACT(EPOCH FROM tree_modified_at)::bigint
|
||||
FROM storage.folders
|
||||
WHERE id = ANY($1) AND NOT is_trashed
|
||||
"#,
|
||||
)
|
||||
.bind(&uuid_ids)
|
||||
.fetch_all(self.pool())
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("FolderDb", format!("get_folders_by_ids: {e}")))?;
|
||||
|
||||
rows.into_iter()
|
||||
.map(|r| Self::row_to_folder(r.0, r.1, r.2, r.3, Some(r.4), r.5, r.6, r.7))
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
|
||||
impl FolderRepository for FolderDbRepository {
|
||||
|
||||
@@ -11,8 +11,8 @@ use axum::{
|
||||
http::StatusCode,
|
||||
response::IntoResponse,
|
||||
};
|
||||
use futures::future::join_all;
|
||||
use serde::Deserialize;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
use tracing::{error, warn};
|
||||
use utoipa::IntoParams;
|
||||
@@ -26,13 +26,10 @@ use crate::application::dtos::grant_dto::{
|
||||
SubjectInputDto, UpdateRoleDto, role_from_permissions,
|
||||
};
|
||||
use crate::application::ports::authorization_ports::AuthorizationEngine;
|
||||
use crate::application::ports::file_ports::FileRetrievalUseCase;
|
||||
use crate::application::ports::folder_ports::FolderUseCase;
|
||||
use crate::application::services::recipient_notification_service::NotifyTrigger;
|
||||
use crate::common::di::AppState;
|
||||
#[allow(unused_imports)]
|
||||
use crate::common::errors::DomainError;
|
||||
use crate::domain::errors::ErrorKind;
|
||||
use crate::domain::services::authorization::{
|
||||
GrantCursor, IncomingGrantSummary, OutgoingResourceSummary, Permission, Resource, ResourceKind,
|
||||
Role, Subject,
|
||||
@@ -664,83 +661,64 @@ pub async fn list_shared_with_me(
|
||||
.map(|s| s.resource_id.to_string())
|
||||
.collect();
|
||||
|
||||
// Resolve resource details concurrently (files and folders in parallel).
|
||||
let (file_results, folder_results) = tokio::join!(
|
||||
join_all(file_ids.iter().map(|id| file_service.get_file(id))),
|
||||
join_all(folder_ids.iter().map(|id| folder_service.get_folder(id)))
|
||||
// Resolve resource details in two batch queries (was one per id via
|
||||
// join_all, which could fan out to ~limit concurrent pooled connections
|
||||
// and starve the primary pool). Missing ids — stale grants whose resource
|
||||
// was deleted before the cascade trigger fired — drop out of the maps.
|
||||
let (file_list, folder_list) = tokio::join!(
|
||||
file_service.get_files_by_ids(&file_ids),
|
||||
folder_service.get_folders_by_ids(&folder_ids)
|
||||
);
|
||||
let file_map: HashMap<String, _> = match file_list {
|
||||
Ok(files) => files.into_iter().map(|f| (f.id.clone(), f)).collect(),
|
||||
Err(e) => return AppError::from(e).into_response(),
|
||||
};
|
||||
let folder_map: HashMap<String, _> = match folder_list {
|
||||
Ok(folders) => folders.into_iter().map(|f| (f.id.clone(), f)).collect(),
|
||||
Err(e) => return AppError::from(e).into_response(),
|
||||
};
|
||||
|
||||
// Build the unified item list in original grant order (newest first).
|
||||
// We iterate summaries in order and pick the resolved result from the
|
||||
// appropriate typed bucket.
|
||||
let mut file_idx = 0usize;
|
||||
let mut folder_idx = 0usize;
|
||||
|
||||
// Build the unified item list in original grant order (newest first),
|
||||
// looking each resolved resource up by id.
|
||||
let mut items: Vec<SharedWithMeItemDto> = Vec::with_capacity(summaries.len());
|
||||
|
||||
for summary in &summaries {
|
||||
let rid = summary.resource_id.to_string();
|
||||
match summary.resource_type {
|
||||
ResourceKind::File => {
|
||||
let result = &file_results[file_idx];
|
||||
file_idx += 1;
|
||||
match result {
|
||||
Ok(file_dto) => {
|
||||
items.push(SharedWithMeItemDto {
|
||||
resource_type: ResourceTypeDto::File,
|
||||
permissions: summary.permissions.iter().map(|p| (*p).into()).collect(),
|
||||
granted_at: summary.granted_at,
|
||||
granted_by: summary.granted_by,
|
||||
resource: ResourceContentDto::File(
|
||||
file_dto.clone().without_hierarchy_info(),
|
||||
),
|
||||
});
|
||||
}
|
||||
Err(e) if e.kind == ErrorKind::NotFound => {
|
||||
// Stale grant (file deleted, trigger not yet fired) — skip silently.
|
||||
warn!(
|
||||
"Skipping stale file grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
);
|
||||
}
|
||||
Err(e) => {
|
||||
return AppError::internal_error(format!(
|
||||
"Failed to fetch file {}: {e}",
|
||||
summary.resource_id
|
||||
))
|
||||
.into_response();
|
||||
}
|
||||
ResourceKind::File => match file_map.get(&rid) {
|
||||
Some(file_dto) => {
|
||||
items.push(SharedWithMeItemDto {
|
||||
resource_type: ResourceTypeDto::File,
|
||||
permissions: summary.permissions.iter().map(|p| (*p).into()).collect(),
|
||||
granted_at: summary.granted_at,
|
||||
granted_by: summary.granted_by,
|
||||
resource: ResourceContentDto::File(
|
||||
file_dto.clone().without_hierarchy_info(),
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
ResourceKind::Folder => {
|
||||
let result = &folder_results[folder_idx];
|
||||
folder_idx += 1;
|
||||
match result {
|
||||
Ok(folder_dto) => {
|
||||
items.push(SharedWithMeItemDto {
|
||||
resource_type: ResourceTypeDto::Folder,
|
||||
permissions: summary.permissions.iter().map(|p| (*p).into()).collect(),
|
||||
granted_at: summary.granted_at,
|
||||
granted_by: summary.granted_by,
|
||||
resource: ResourceContentDto::Folder(
|
||||
folder_dto.clone().without_hierarchy_info(),
|
||||
),
|
||||
});
|
||||
}
|
||||
Err(e) if e.kind == ErrorKind::NotFound => {
|
||||
warn!(
|
||||
"Skipping stale folder grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
);
|
||||
}
|
||||
Err(e) => {
|
||||
return AppError::internal_error(format!(
|
||||
"Failed to fetch folder {}: {e}",
|
||||
summary.resource_id
|
||||
))
|
||||
.into_response();
|
||||
}
|
||||
None => warn!(
|
||||
"Skipping stale file grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
),
|
||||
},
|
||||
ResourceKind::Folder => match folder_map.get(&rid) {
|
||||
Some(folder_dto) => {
|
||||
items.push(SharedWithMeItemDto {
|
||||
resource_type: ResourceTypeDto::Folder,
|
||||
permissions: summary.permissions.iter().map(|p| (*p).into()).collect(),
|
||||
granted_at: summary.granted_at,
|
||||
granted_by: summary.granted_by,
|
||||
resource: ResourceContentDto::Folder(
|
||||
folder_dto.clone().without_hierarchy_info(),
|
||||
),
|
||||
});
|
||||
}
|
||||
}
|
||||
None => warn!(
|
||||
"Skipping stale folder grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -909,13 +887,20 @@ pub async fn list_my_shares(
|
||||
.map(|s| s.resource_id.to_string())
|
||||
.collect();
|
||||
|
||||
let (file_results, folder_results) = tokio::join!(
|
||||
join_all(file_ids.iter().map(|id| file_service.get_file(id))),
|
||||
join_all(folder_ids.iter().map(|id| folder_service.get_folder(id)))
|
||||
// Two batch queries instead of one get_* per id (see list_shared_with_me).
|
||||
let (file_list, folder_list) = tokio::join!(
|
||||
file_service.get_files_by_ids(&file_ids),
|
||||
folder_service.get_folders_by_ids(&folder_ids)
|
||||
);
|
||||
let file_map: HashMap<String, _> = match file_list {
|
||||
Ok(files) => files.into_iter().map(|f| (f.id.clone(), f)).collect(),
|
||||
Err(e) => return AppError::from(e).into_response(),
|
||||
};
|
||||
let folder_map: HashMap<String, _> = match folder_list {
|
||||
Ok(folders) => folders.into_iter().map(|f| (f.id.clone(), f)).collect(),
|
||||
Err(e) => return AppError::from(e).into_response(),
|
||||
};
|
||||
|
||||
let mut file_idx = 0usize;
|
||||
let mut folder_idx = 0usize;
|
||||
let mut items: Vec<OutgoingResourceItemDto> = Vec::with_capacity(summaries.len());
|
||||
|
||||
for summary in &summaries {
|
||||
@@ -935,64 +920,39 @@ pub async fn list_my_shares(
|
||||
})
|
||||
.collect();
|
||||
|
||||
let rid = summary.resource_id.to_string();
|
||||
match summary.resource_type {
|
||||
ResourceKind::File => {
|
||||
let result = &file_results[file_idx];
|
||||
file_idx += 1;
|
||||
match result {
|
||||
Ok(file_dto) => {
|
||||
// Caller is the granter — they had share-access to the
|
||||
// resource, so the containing hierarchy is already known
|
||||
// to them. Keep `path` (unlike list_shared_with_me).
|
||||
items.push(OutgoingResourceItemDto {
|
||||
resource_type: ResourceTypeDto::File,
|
||||
first_shared_at: summary.first_shared_at,
|
||||
resource: ResourceContentDto::File(file_dto.clone()),
|
||||
grants,
|
||||
});
|
||||
}
|
||||
Err(e) if e.kind == ErrorKind::NotFound => {
|
||||
warn!(
|
||||
"Skipping stale outgoing file grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
);
|
||||
}
|
||||
Err(e) => {
|
||||
return AppError::internal_error(format!(
|
||||
"Failed to fetch file {}: {e}",
|
||||
summary.resource_id
|
||||
))
|
||||
.into_response();
|
||||
}
|
||||
ResourceKind::File => match file_map.get(&rid) {
|
||||
Some(file_dto) => {
|
||||
// Caller is the granter — they had share-access to the
|
||||
// resource, so the containing hierarchy is already known
|
||||
// to them. Keep `path` (unlike list_shared_with_me).
|
||||
items.push(OutgoingResourceItemDto {
|
||||
resource_type: ResourceTypeDto::File,
|
||||
first_shared_at: summary.first_shared_at,
|
||||
resource: ResourceContentDto::File(file_dto.clone()),
|
||||
grants,
|
||||
});
|
||||
}
|
||||
}
|
||||
ResourceKind::Folder => {
|
||||
let result = &folder_results[folder_idx];
|
||||
folder_idx += 1;
|
||||
match result {
|
||||
Ok(folder_dto) => {
|
||||
items.push(OutgoingResourceItemDto {
|
||||
resource_type: ResourceTypeDto::Folder,
|
||||
first_shared_at: summary.first_shared_at,
|
||||
resource: ResourceContentDto::Folder(folder_dto.clone()),
|
||||
grants,
|
||||
});
|
||||
}
|
||||
Err(e) if e.kind == ErrorKind::NotFound => {
|
||||
warn!(
|
||||
"Skipping stale outgoing folder grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
);
|
||||
}
|
||||
Err(e) => {
|
||||
return AppError::internal_error(format!(
|
||||
"Failed to fetch folder {}: {e}",
|
||||
summary.resource_id
|
||||
))
|
||||
.into_response();
|
||||
}
|
||||
None => warn!(
|
||||
"Skipping stale outgoing file grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
),
|
||||
},
|
||||
ResourceKind::Folder => match folder_map.get(&rid) {
|
||||
Some(folder_dto) => {
|
||||
items.push(OutgoingResourceItemDto {
|
||||
resource_type: ResourceTypeDto::Folder,
|
||||
first_shared_at: summary.first_shared_at,
|
||||
resource: ResourceContentDto::Folder(folder_dto.clone()),
|
||||
grants,
|
||||
});
|
||||
}
|
||||
}
|
||||
None => warn!(
|
||||
"Skipping stale outgoing folder grant for resource_id={}: not found",
|
||||
summary.resource_id
|
||||
),
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -7,7 +7,7 @@ use quick_xml::{
|
||||
Reader, Writer,
|
||||
events::{BytesEnd, BytesStart, Event},
|
||||
};
|
||||
use std::collections::HashSet;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::application::dtos::display_helpers::{
|
||||
@@ -17,7 +17,6 @@ use crate::application::dtos::file_dto::FileDto;
|
||||
use crate::application::dtos::folder_dto::FolderDto;
|
||||
use crate::application::dtos::search_dto::SearchCriteriaDto;
|
||||
use crate::application::ports::favorites_ports::FavoritesUseCase;
|
||||
use crate::application::ports::file_ports::FileRetrievalUseCase;
|
||||
use crate::application::ports::folder_ports::FolderUseCase;
|
||||
use crate::application::ports::inbound::SearchUseCase;
|
||||
use crate::common::di::AppState;
|
||||
@@ -86,20 +85,47 @@ async fn handle_filter_files(
|
||||
|
||||
let home_prefix = format!("My Folder - {}/", user.username);
|
||||
|
||||
// Pass 1: fetch the favorited DTOs (the per-item fetch is a separate
|
||||
// concern from the oc:fileid resolution batched below).
|
||||
// Pass 1: resolve the favorited DTOs in two batch queries (was one
|
||||
// get_* per favorite — up to N serial round-trips on a sync client's
|
||||
// REPORT). Results are looked up by id so the response keeps favorites
|
||||
// order; missing/trashed favorites simply drop out (as before).
|
||||
let mut file_ids: Vec<String> = Vec::new();
|
||||
let mut folder_ids: Vec<String> = Vec::new();
|
||||
for fav in &favorites {
|
||||
match fav.item_type.as_str() {
|
||||
"file" => file_ids.push(fav.item_id.clone()),
|
||||
"folder" => folder_ids.push(fav.item_id.clone()),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
let file_map: HashMap<String, FileDto> = file_service
|
||||
.get_files_by_ids(&file_ids)
|
||||
.await
|
||||
.map_err(|e| AppError::internal_error(format!("Failed to resolve favorite files: {e}")))?
|
||||
.into_iter()
|
||||
.map(|f| (f.id.clone(), f))
|
||||
.collect();
|
||||
let folder_map: HashMap<String, FolderDto> = folder_service
|
||||
.get_folders_by_ids(&folder_ids)
|
||||
.await
|
||||
.map_err(|e| AppError::internal_error(format!("Failed to resolve favorite folders: {e}")))?
|
||||
.into_iter()
|
||||
.map(|f| (f.id.clone(), f))
|
||||
.collect();
|
||||
|
||||
let mut files: Vec<FileDto> = Vec::new();
|
||||
let mut folders: Vec<FolderDto> = Vec::new();
|
||||
for fav in &favorites {
|
||||
match fav.item_type.as_str() {
|
||||
"file" => {
|
||||
if let Ok(f) = file_service.get_file(&fav.item_id).await {
|
||||
files.push(f);
|
||||
if let Some(f) = file_map.get(&fav.item_id) {
|
||||
files.push(f.clone());
|
||||
}
|
||||
}
|
||||
"folder" => {
|
||||
if let Ok(f) = folder_service.get_folder(&fav.item_id).await {
|
||||
folders.push(f);
|
||||
if let Some(f) = folder_map.get(&fav.item_id) {
|
||||
folders.push(f.clone());
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
|
||||
Reference in New Issue
Block a user