diff --git a/src/application/services/file_retrieval_service.rs b/src/application/services/file_retrieval_service.rs index 41f83bd8..27ae48e7 100644 --- a/src/application/services/file_retrieval_service.rs +++ b/src/application/services/file_retrieval_service.rs @@ -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, DomainError> { + let files = self.file_read.get_files_by_ids(ids).await?; + Ok(files.into_iter().map(FileDto::from).collect()) + } } impl FileRetrievalUseCase for FileRetrievalService { diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index 8fd096e8..66f3893f 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -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, 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"). diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index f2fefdfe..4999f101 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -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, DomainError> { + let uuid_ids: Vec = 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::, _>>() + .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. diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index 97852fae..5d38ce56 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -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, DomainError> { + let uuid_ids: Vec = 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 { diff --git a/src/interfaces/api/handlers/grant_handler.rs b/src/interfaces/api/handlers/grant_handler.rs index 7fc39ab3..63674364 100644 --- a/src/interfaces/api/handlers/grant_handler.rs +++ b/src/interfaces/api/handlers/grant_handler.rs @@ -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 = 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 = 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 = 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 = 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 = 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 = 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 + ), + }, } } diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index d5e81b38..9e056d0b 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -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 = Vec::new(); + let mut folder_ids: Vec = 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 = 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 = 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 = Vec::new(); let mut folders: Vec = 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()); } } _ => {}