From 28966ce28ece52a1f79fe70fa657d37574ac94eb Mon Sep 17 00:00:00 2001 From: Dionisio Date: Tue, 24 Feb 2026 17:15:36 +0100 Subject: [PATCH] optimize folder search: SQL-level filtering, user isolation, no in-memory filter; batch cascade trigger --- db/schema.sql | 20 ++- src/application/ports/inbound.rs | 5 +- src/application/services/search_service.rs | 61 +++---- src/common/config.rs | 24 ++- src/common/di.rs | 6 +- src/common/stubs.rs | 2 +- src/domain/repositories/folder_repository.rs | 35 ++++ src/infrastructure/auth_factory.rs | 8 +- .../repositories/pg/folder_db_repository.rs | 160 +++++++++++++++++- .../services/password_hasher.rs | 63 +++++-- src/interfaces/api/handlers/file_handler.rs | 101 +++++------ src/interfaces/api/handlers/search_handler.rs | 7 +- 12 files changed, 361 insertions(+), 131 deletions(-) diff --git a/db/schema.sql b/db/schema.sql index 9d2883bb..e8e17351 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -402,15 +402,27 @@ CREATE OR REPLACE TRIGGER trg_folders_path BEFORE INSERT OR UPDATE OF name, parent_id ON storage.folders FOR EACH ROW EXECUTE FUNCTION storage.compute_folder_path(); --- ── Cascade trigger: when a folder's path/lpath changes, update all descendants ── +-- ── Cascade trigger: when a folder's path/lpath changes, update ALL descendants +-- in a single batch UPDATE using the GiST index on lpath. +-- pg_trigger_depth() guard prevents re-firing on the rows touched by the +-- batch UPDATE itself (they also change path/lpath, which would otherwise +-- cause infinite recursion). CREATE OR REPLACE FUNCTION storage.cascade_folder_path() RETURNS trigger AS $$ BEGIN + IF pg_trigger_depth() > 1 THEN + RETURN NEW; + END IF; + IF OLD.path IS DISTINCT FROM NEW.path OR OLD.lpath IS DISTINCT FROM NEW.lpath THEN - -- Update all descendant folders (recursive via trigger re-fire) + -- Single batch update: rewrite path/lpath for every descendant at once. + -- Uses the GiST index idx_folders_lpath for the <@ operator. + -- Does NOT touch name or parent_id, so compute_folder_path does not fire. UPDATE storage.folders - SET parent_id = parent_id -- no-op value change, but fires the BEFORE UPDATE trigger - WHERE parent_id = NEW.id; + SET path = NEW.path || substr(path, length(OLD.path) + 1), + lpath = NEW.lpath || subpath(lpath, nlevel(OLD.lpath)) + WHERE lpath <@ OLD.lpath + AND id != NEW.id; END IF; RETURN NEW; END; diff --git a/src/application/ports/inbound.rs b/src/application/ports/inbound.rs index 444f1ca0..027cd3d3 100644 --- a/src/application/ports/inbound.rs +++ b/src/application/ports/inbound.rs @@ -94,7 +94,10 @@ pub trait FolderUseCase: Send + Sync + 'static { #[async_trait] pub trait SearchUseCase: Send + Sync + 'static { /// Performs a full search based on the specified criteria. - async fn search(&self, criteria: SearchCriteriaDto) -> Result; + /// + /// `user_id` identifies the authenticated user so that SQL queries filter + /// by owner and the result cache is isolated per tenant. + async fn search(&self, criteria: SearchCriteriaDto, user_id: &str) -> Result; /// Returns quick suggestions for autocomplete (lightweight, fast). async fn suggest( diff --git a/src/application/services/search_service.rs b/src/application/services/search_service.rs index 25e3417b..ec0447a4 100644 --- a/src/application/services/search_service.rs +++ b/src/application/services/search_service.rs @@ -272,12 +272,9 @@ impl SearchUseCase for SearchService { * - Human-readable size formatting * - Pagination */ - async fn search(&self, criteria: SearchCriteriaDto) -> Result { + async fn search(&self, criteria: SearchCriteriaDto, user_id: &str) -> Result { let start = Instant::now(); - // TODO: Get user ID from the authentication context - let user_id = "default-user"; - // Try to get from cache let cache_key = Self::create_cache_key(&criteria, user_id); if let Some(cached_results) = self.get_from_cache(cache_key).await { @@ -302,27 +299,19 @@ impl SearchUseCase for SearchService { .map(|f| Self::enrich_file(f, query)) .collect(); - // Get folders for this folder (non-recursive) + // Get folders for this folder (non-recursive, filtered in SQL) let folders = self .folder_repository - .list_folders(criteria.folder_id.as_deref()) + .search_folders( + criteria.folder_id.as_deref(), + criteria.name_contains.as_deref(), + user_id, + false, + ) .await?; - // Filter folders if name criteria present - let filtered_folders: Vec = if let Some(name_query) = &criteria.name_contains - { - let query_lower = name_query.to_lowercase(); - folders - .into_iter() - .map(FolderDto::from) - .filter(|f| { - let folder_name_lower = f.name.to_lowercase(); - folder_name_lower.contains(&query_lower) - }) - .collect() - } else { - folders.into_iter().map(FolderDto::from).collect() - }; + let filtered_folders: Vec = + folders.into_iter().map(FolderDto::from).collect(); // For folders, apply sorting and pagination in memory (usually fewer folders) let mut enriched_folders: Vec = filtered_folders @@ -397,24 +386,16 @@ impl SearchUseCase for SearchService { .search_files_in_subtree(criteria.folder_id.as_deref(), &criteria, user_id) .await?; - // Get descendant folders (ltree-based when folder_id is specified) - let found_folders: Vec = if let Some(ref fid) = criteria.folder_id { - self.folder_repository - .list_descendant_folders(fid, criteria.name_contains.as_deref(), user_id) - .await? - } else { - // No folder scope → search all user folders - let all_folders = self.folder_repository.list_folders(None).await?; - if let Some(ref name_query) = criteria.name_contains { - let q = name_query.to_lowercase(); - all_folders - .into_iter() - .filter(|f| f.name().to_lowercase().contains(&q)) - .collect() - } else { - all_folders - } - }; + // Get folders (SQL-filtered, user-scoped, recursive when applicable) + let found_folders: Vec = self + .folder_repository + .search_folders( + criteria.folder_id.as_deref(), + criteria.name_contains.as_deref(), + user_id, + true, + ) + .await?; // ── Convert to DTOs and enrich with server-computed metadata ── let file_dtos: Vec = found_files.into_iter().map(FileDto::from).collect(); @@ -515,7 +496,7 @@ impl SearchService { #[async_trait] impl SearchUseCase for SearchServiceStub { - async fn search(&self, _criteria: SearchCriteriaDto) -> Result { + async fn search(&self, _criteria: SearchCriteriaDto, _user_id: &str) -> Result { Ok(SearchResultsDto::empty()) } diff --git a/src/common/config.rs b/src/common/config.rs index 84c8d08d..34f9e49e 100644 --- a/src/common/config.rs +++ b/src/common/config.rs @@ -248,8 +248,12 @@ pub struct AuthConfig { pub jwt_secret: String, pub access_token_expiry_secs: i64, pub refresh_token_expiry_secs: i64, + /// Argon2id memory cost in KiB (default 65536 = 64 MiB) pub hash_memory_cost: u32, + /// Argon2id time cost / iterations (default 3) pub hash_time_cost: u32, + /// Argon2id parallelism lanes (default 2) + pub hash_parallelism: u32, } impl Default for AuthConfig { @@ -261,8 +265,9 @@ impl Default for AuthConfig { jwt_secret: String::new(), access_token_expiry_secs: 3600, // 1 hour refresh_token_expiry_secs: 2592000, // 30 days - hash_memory_cost: 65536, // 64MB + hash_memory_cost: 65536, // 64 MiB hash_time_cost: 3, + hash_parallelism: 2, } } } @@ -537,6 +542,23 @@ impl AppConfig { config.auth.refresh_token_expiry_secs = val; } + // Argon2 hashing parameters + if let Ok(v) = env::var("OXICLOUD_HASH_MEMORY_COST").map(|v| v.parse::()) + && let Ok(val) = v + { + config.auth.hash_memory_cost = val; + } + if let Ok(v) = env::var("OXICLOUD_HASH_TIME_COST").map(|v| v.parse::()) + && let Ok(val) = v + { + config.auth.hash_time_cost = val; + } + if let Ok(v) = env::var("OXICLOUD_HASH_PARALLELISM").map(|v| v.parse::()) + && let Ok(val) = v + { + config.auth.hash_parallelism = val; + } + // Feature flags if let Ok(enable_auth) = env::var("OXICLOUD_ENABLE_AUTH").map(|v| v.parse::()) && let Ok(val) = enable_auth diff --git a/src/common/di.rs b/src/common/di.rs index 33e7c8a2..73447b97 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -327,7 +327,11 @@ impl AppServiceFactory { // Build a password hasher for share password verification let password_hasher: Arc = - Arc::new(crate::infrastructure::services::password_hasher::Argon2PasswordHasher::new()); + Arc::new(crate::infrastructure::services::password_hasher::Argon2PasswordHasher::new( + self.config.auth.hash_memory_cost, + self.config.auth.hash_time_cost, + self.config.auth.hash_parallelism, + )); let service = Arc::new(ShareService::new( Arc::new(self.config.clone()), diff --git a/src/common/stubs.rs b/src/common/stubs.rs index ebcf9c01..3099a1ca 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -621,7 +621,7 @@ pub struct StubSearchUseCase; #[async_trait] impl SearchUseCase for StubSearchUseCase { - async fn search(&self, _criteria: SearchCriteriaDto) -> Result { + async fn search(&self, _criteria: SearchCriteriaDto, _user_id: &str) -> Result { Ok(SearchResultsDto::empty()) } diff --git a/src/domain/repositories/folder_repository.rs b/src/domain/repositories/folder_repository.rs index b1b9267e..5ac2c0d4 100644 --- a/src/domain/repositories/folder_repository.rs +++ b/src/domain/repositories/folder_repository.rs @@ -135,6 +135,41 @@ pub trait FolderRepository: Send + Sync + 'static { Ok(Vec::new()) } + /// Search folders with SQL-level filtering by name, user, and scope. + /// + /// - **Non-recursive** (`recursive = false`): searches direct children of + /// `parent_id` (or root folders when `None`). + /// - **Recursive with `parent_id`**: delegates to `list_descendant_folders` + /// (ltree GiST-indexed scan). + /// - **Recursive without `parent_id`**: searches ALL folders owned by + /// `user_id` with optional name filter in SQL. + /// + /// The default implementation falls back to `list_folders` + in-memory + /// filter so that stubs and mocks compile without changes. + async fn search_folders( + &self, + parent_id: Option<&str>, + name_contains: Option<&str>, + user_id: &str, + recursive: bool, + ) -> Result, DomainError> { + // Recursive with folder_id → use optimised ltree scan + if recursive { + if let Some(fid) = parent_id { + return self.list_descendant_folders(fid, name_contains, user_id).await; + } + } + // Fallback: load + filter in memory (stubs / mocks) + let all = self.list_folders(parent_id).await?; + match name_contains { + Some(q) if !q.is_empty() => { + let q = q.to_lowercase(); + Ok(all.into_iter().filter(|f| f.name().to_lowercase().contains(&q)).collect()) + } + _ => Ok(all), + } + } + /// Return up to `limit` folders whose name contains `query` (case-insensitive). /// /// Results are ordered by relevance (exact > starts-with > contains) for diff --git a/src/infrastructure/auth_factory.rs b/src/infrastructure/auth_factory.rs index 2f3bb0db..ff35dba4 100644 --- a/src/infrastructure/auth_factory.rs +++ b/src/infrastructure/auth_factory.rs @@ -24,8 +24,12 @@ pub async fn create_auth_services( config.auth.refresh_token_expiry_secs, )); - // Create password hashing service - let password_hasher = Arc::new(Argon2PasswordHasher::new()); + // Create password hashing service with configured Argon2id parameters + let password_hasher = Arc::new(Argon2PasswordHasher::new( + config.auth.hash_memory_cost, + config.auth.hash_time_cost, + config.auth.hash_parallelism, + )); // Create PostgreSQL repositories let user_repository = Arc::new(UserPgRepository::new(pool.clone())); diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index 3f952516..3f2d25d2 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -414,8 +414,9 @@ impl FolderRepository for FolderDbRepository { } async fn rename_folder(&self, id: &str, new_name: String) -> Result { - // The BEFORE UPDATE trigger on `name` will recompute path/lpath - // and cascade to descendants automatically. + // The BEFORE UPDATE trigger recomputes path/lpath for this row; + // the AFTER UPDATE cascade trigger then batch-updates all + // descendants in a single UPDATE using the GiST lpath index. sqlx::query( r#" UPDATE storage.folders @@ -444,8 +445,9 @@ impl FolderRepository for FolderDbRepository { id: &str, new_parent_id: Option<&str>, ) -> Result { - // The BEFORE UPDATE trigger on `parent_id` will recompute path/lpath - // and cascade to descendants automatically. + // The BEFORE UPDATE trigger recomputes path/lpath for this row; + // the AFTER UPDATE cascade trigger then batch-updates all + // descendants in a single UPDATE using the GiST lpath index. sqlx::query( r#" UPDATE storage.folders @@ -561,8 +563,9 @@ impl FolderRepository for FolderDbRepository { // Only restore the folder itself. // Child files were never marked as trashed — they become visible // again automatically once their parent folder is un-trashed. - // The BEFORE UPDATE trigger on parent_id will recompute path/lpath - // automatically when original_parent_id is restored. + // The BEFORE UPDATE trigger recomputes path/lpath when + // original_parent_id is restored; the cascade trigger + // batch-updates all descendants via the GiST lpath index. let result = sqlx::query_scalar::<_, i64>( r#" WITH restore_folder AS ( @@ -711,6 +714,151 @@ impl FolderRepository for FolderDbRepository { .collect() } + /// SQL-level folder search with name filter, user isolation, and + /// recursive / non-recursive modes. + /// + /// - Non-recursive: `WHERE parent_id = $1 AND user_id = $2 [AND LIKE]` + /// - Recursive + folder_id: delegates to `list_descendant_folders` + /// - Recursive + no folder_id: `WHERE user_id = $1 [AND LIKE]` + async fn search_folders( + &self, + parent_id: Option<&str>, + name_contains: Option<&str>, + user_id: &str, + recursive: bool, + ) -> Result, DomainError> { + // Recursive with folder scope → existing optimised ltree scan + if recursive { + if let Some(fid) = parent_id { + return self.list_descendant_folders(fid, name_contains, user_id).await; + } + } + + // Build optional name filter + let (name_clause, name_pattern) = match name_contains { + Some(name) if !name.is_empty() => ( + if recursive { + " AND LOWER(fo.name) LIKE $2" + } else { + " AND LOWER(fo.name) LIKE $3" + }, + Some(format!("%{}%", name.to_lowercase())), + ), + _ => ("", None), + }; + + if recursive { + // Recursive, no folder scope → ALL user folders + let sql = format!( + "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ + fo.user_id::text, \ + EXTRACT(EPOCH FROM fo.created_at)::bigint, \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + FROM storage.folders fo \ + WHERE fo.user_id = $1 \ + AND fo.is_trashed = false \ + {name_clause} \ + ORDER BY fo.name" + ); + + let rows: Vec<(String, String, String, Option, Option, i64, i64)> = + if let Some(ref pattern) = name_pattern { + sqlx::query_as(&sql) + .bind(user_id) + .bind(pattern) + .fetch_all(self.pool()) + .await + } else { + sqlx::query_as(&sql) + .bind(user_id) + .fetch_all(self.pool()) + .await + } + .map_err(|e| { + DomainError::internal_error("FolderDb", format!("search_folders: {e}")) + })?; + + return rows + .into_iter() + .map(|(id, name, path, pid, uid, ca, ma)| { + Self::row_to_folder(id, name, path, pid, uid, ca, ma) + }) + .collect(); + } + + // Non-recursive: direct children of parent_id, filtered by user + let sql = if parent_id.is_some() { + format!( + "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ + fo.user_id::text, \ + EXTRACT(EPOCH FROM fo.created_at)::bigint, \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + FROM storage.folders fo \ + WHERE fo.parent_id = $1::uuid \ + AND fo.user_id = $2 \ + AND fo.is_trashed = false \ + {name_clause} \ + ORDER BY fo.name" + ) + } else { + // Root folders: parent_id IS NULL, reindex params ($1=user_id, $2=pattern) + let name_clause_root = match name_contains { + Some(name) if !name.is_empty() => " AND LOWER(fo.name) LIKE $2", + _ => "", + }; + format!( + "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ + fo.user_id::text, \ + EXTRACT(EPOCH FROM fo.created_at)::bigint, \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint \ + FROM storage.folders fo \ + WHERE fo.parent_id IS NULL \ + AND fo.user_id = $1 \ + AND fo.is_trashed = false \ + {name_clause_root} \ + ORDER BY fo.name" + ) + }; + + let rows: Vec<(String, String, String, Option, Option, i64, i64)> = + if let Some(pid) = parent_id { + if let Some(ref pattern) = name_pattern { + sqlx::query_as(&sql) + .bind(pid) + .bind(user_id) + .bind(pattern) + .fetch_all(self.pool()) + .await + } else { + sqlx::query_as(&sql) + .bind(pid) + .bind(user_id) + .fetch_all(self.pool()) + .await + } + } else if let Some(ref pattern) = name_pattern { + sqlx::query_as(&sql) + .bind(user_id) + .bind(pattern) + .fetch_all(self.pool()) + .await + } else { + sqlx::query_as(&sql) + .bind(user_id) + .fetch_all(self.pool()) + .await + } + .map_err(|e| { + DomainError::internal_error("FolderDb", format!("search_folders: {e}")) + })?; + + rows.into_iter() + .map(|(id, name, path, pid, uid, ca, ma)| { + Self::row_to_folder(id, name, path, pid, uid, ca, ma) + }) + .collect() + } + /// Lists all descendant folders in a subtree using ltree GiST index. /// /// Single SQL query: `fo.lpath <@ (root's lpath)` fetches the entire diff --git a/src/infrastructure/services/password_hasher.rs b/src/infrastructure/services/password_hasher.rs index 4bef4ca2..78bb5a5e 100644 --- a/src/infrastructure/services/password_hasher.rs +++ b/src/infrastructure/services/password_hasher.rs @@ -3,12 +3,15 @@ //! This module provides a secure password hashing implementation using the Argon2id //! algorithm, which is the recommended choice for password hashing as of 2023+. //! -//! Both `hash_password` and `verify_password` are CPU-intensive (~300-500 ms with -//! default parameters) so they run inside `spawn_blocking` to avoid blocking Tokio -//! worker threads. +//! Both `hash_password` and `verify_password` are CPU-intensive so they run inside +//! `spawn_blocking` to avoid blocking Tokio worker threads. +//! +//! The Argon2id parameters (`m_cost`, `t_cost`, `p_cost`) are injected at +//! construction time from `AuthConfig`, so operators can tune security vs. +//! latency via environment variables. use argon2::password_hash::SaltString; -use argon2::{Argon2, PasswordHash, PasswordHasher, PasswordVerifier}; +use argon2::{Algorithm, Argon2, Params, PasswordHash, PasswordHasher, PasswordVerifier, Version}; use async_trait::async_trait; use rand_core::OsRng; @@ -20,23 +23,38 @@ use crate::common::errors::{DomainError, ErrorKind}; /// Uses Argon2id algorithm which provides resistance against both side-channel /// and GPU-based attacks. This is the recommended algorithm for password hashing. /// -/// The struct is stateless — `Argon2::default()` is constructed per call inside -/// `spawn_blocking` so it is `Send` without extra synchronisation. +/// The struct stores the validated `Params` so that `Argon2` can be cheaply +/// reconstructed inside each `spawn_blocking` call (it is not `Send`). #[derive(Debug, Clone)] pub struct Argon2PasswordHasher { - _private: (), + params: Params, } impl Argon2PasswordHasher { - /// Create a new Argon2PasswordHasher with default secure parameters. - pub fn new() -> Self { - Self { _private: () } - } -} + /// Create a new hasher with explicit Argon2id parameters. + /// + /// - `memory_cost`: memory in KiB (e.g. 65536 = 64 MiB) + /// - `time_cost`: number of iterations (e.g. 3) + /// - `parallelism`: lanes of parallelism (e.g. 2) + /// + /// Panics at startup if the parameters are invalid (caught immediately). + pub fn new(memory_cost: u32, time_cost: u32, parallelism: u32) -> Self { + let params = Params::new(memory_cost, time_cost, parallelism, None) + .unwrap_or_else(|e| { + panic!( + "Invalid Argon2 parameters (m={}, t={}, p={}): {}", + memory_cost, time_cost, parallelism, e + ) + }); -impl Default for Argon2PasswordHasher { - fn default() -> Self { - Self::new() + tracing::info!( + "Argon2PasswordHasher initialized: m_cost={} KiB, t_cost={}, p_cost={}", + memory_cost, + time_cost, + parallelism, + ); + + Self { params } } } @@ -44,9 +62,11 @@ impl Default for Argon2PasswordHasher { impl PasswordHasherPort for Argon2PasswordHasher { async fn hash_password(&self, password: &str) -> Result { let pwd = password.to_owned(); + let params = self.params.clone(); tokio::task::spawn_blocking(move || { let salt = SaltString::generate(&mut OsRng); - Argon2::default() + let argon2 = Argon2::new(Algorithm::Argon2id, Version::V0x13, params); + argon2 .hash_password(pwd.as_bytes(), &salt) .map(|hash| hash.to_string()) .map_err(|e| { @@ -79,6 +99,8 @@ impl PasswordHasherPort for Argon2PasswordHasher { ) })?; + // verify_password reads m/t/p from the hash string itself, + // so existing hashes (with old params) verify correctly. Ok(Argon2::default() .verify_password(pwd.as_bytes(), &parsed_hash) .is_ok()) @@ -98,9 +120,14 @@ impl PasswordHasherPort for Argon2PasswordHasher { mod tests { use super::*; + /// Test params: small values so tests are fast (~10 ms instead of ~400 ms) + fn test_hasher() -> Argon2PasswordHasher { + Argon2PasswordHasher::new(16384, 1, 1) + } + #[tokio::test] async fn test_hash_and_verify_password() { - let hasher = Argon2PasswordHasher::new(); + let hasher = test_hasher(); let password = "test_password_123"; let hash = hasher @@ -123,7 +150,7 @@ mod tests { #[tokio::test] async fn test_different_hashes_for_same_password() { - let hasher = Argon2PasswordHasher::new(); + let hasher = test_hasher(); let password = "same_password"; let hash1 = hasher.hash_password(password).await.expect("Should hash"); diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index 62f33d55..ad01dccb 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -43,8 +43,24 @@ impl FileHandler { pub async fn upload_file( State(state): State, auth_user: AuthUser, - mut multipart: Multipart, + multipart: Multipart, ) -> impl IntoResponse { + match Self::upload_file_inner(&state, &auth_user, multipart).await { + Ok(file) => Self::created_json_response(&file).into_response(), + Err(response) => response.into_response(), + } + } + + /// Core upload logic shared by [`Self::upload_file`] and + /// [`Self::upload_file_with_thumbnails`]. + /// + /// Returns the typed `FileDto` on success so callers can use it + /// directly (e.g. for thumbnail generation) without re-parsing JSON. + async fn upload_file_inner( + state: &GlobalState, + auth_user: &AuthUser, + mut multipart: Multipart, + ) -> Result> { use sha2::{Digest, Sha256}; let upload_service = &state.applications.file_upload_service; @@ -83,9 +99,6 @@ impl FileHandler { .to_string(); // ── Early quota check (before spooling to disk) ────── - // Use the multipart field's Content-Length header if present. - // If the user is already over quota, reject immediately - // without wasting I/O on spooling the entire body. if let Some(storage_svc) = state.storage_usage_service.as_ref() { let estimated_size = field .headers() @@ -103,7 +116,7 @@ impl FileHandler { filename, estimated_size ); - return Self::quota_error_response(err).into_response(); + return Err(Self::quota_error_response(err)); } } @@ -149,22 +162,18 @@ impl FileHandler { if let Err(e) = spool_result { let _ = tokio::fs::remove_file(&temp_path).await; tracing::error!("❌ UPLOAD SPOOL FAILED: {} - {}", filename, e); - return Self::domain_error_response( + return Err(Self::domain_error_response( crate::common::errors::DomainError::internal_error("FileUpload", e), - ) - .into_response(); + )); } // Empty file — use in-memory path if total_size == 0 { let _ = tokio::fs::remove_file(&temp_path).await; - return match upload_service + return upload_service .upload_file(filename, folder_id, content_type, vec![]) .await - { - Ok(file) => Self::created_json_response(&file).into_response(), - Err(err) => Self::domain_error_response(err).into_response(), - }; + .map_err(Self::domain_error_response); } // Finalize hash @@ -183,7 +192,7 @@ impl FileHandler { filename, total_size ); - return Self::quota_error_response(err).into_response(); + return Err(Self::quota_error_response(err)); } // ── Streaming upload (temp file → blob store, hash pre-computed) ─ @@ -205,24 +214,24 @@ impl FileHandler { total_size, file.id ); - return Self::created_json_response(&file).into_response(); + return Ok(file); } Err(err) => { let _ = tokio::fs::remove_file(&temp_path).await; tracing::error!("❌ UPLOAD FAILED: {} - {}", filename, err); - return Self::domain_error_response(err).into_response(); + return Err(Self::domain_error_response(err)); } } } } - ( + Err(( StatusCode::BAD_REQUEST, Json(serde_json::json!({ "error": "No file provided" })), ) - .into_response() + .into_response()) } // ═══════════════════════════════════════════════════════════════════════ @@ -570,52 +579,34 @@ impl FileHandler { /// Uploads a file and generates thumbnails in the background for images. /// - /// Delegates to [`Self::upload_file`] (streaming) and, on success, spawns - /// a background task to generate all thumbnail sizes. + /// Delegates to [`Self::upload_file_inner`] and, on success, spawns + /// a background task to generate all thumbnail sizes before serialising + /// the `FileDto` once. pub async fn upload_file_with_thumbnails( State(state): State, auth_user: AuthUser, multipart: Multipart, ) -> impl IntoResponse { - // Use the streaming upload handler - let response = Self::upload_file(State(state.clone()), auth_user, multipart).await; + let file = match Self::upload_file_inner(&state, &auth_user, multipart).await { + Ok(f) => f, + Err(response) => return response.into_response(), + }; - // Try to extract file info for thumbnail generation - if let Ok(body_bytes) = - axum::body::to_bytes(response.into_response().into_body(), 10 * 1024).await - && let Ok(file_info) = serde_json::from_slice::(&body_bytes) - && let (Some(file_id), Some(mime_type), Some(file_path_str)) = ( - file_info.get("id").and_then(|v| v.as_str()), - file_info.get("mime_type").and_then(|v| v.as_str()), - file_info.get("path").and_then(|v| v.as_str()), - ) - { - // Generate thumbnails for images in background - if state.core.thumbnail_service.is_supported_image(mime_type) { - let file_id = file_id.to_string(); - let file_path_rel = file_path_str.to_string(); - let thumbnail_service = state.core.thumbnail_service.clone(); - let path_service = state.core.path_service.clone(); + // Generate thumbnails for supported images in background + if state.core.thumbnail_service.is_supported_image(&file.mime_type) { + let file_id = file.id.clone(); + let file_path_rel = file.path.clone(); + let thumbnail_service = state.core.thumbnail_service.clone(); + let path_service = state.core.path_service.clone(); - tokio::spawn(async move { - let file_path = path_service.get_root_path().join(&file_path_rel); - tracing::info!("🖼️ Generating thumbnails for: {}", file_id); - thumbnail_service.generate_all_sizes_background(file_id, file_path); - }); - } - - // Return the response - return Response::builder() - .status(StatusCode::CREATED) - .header(header::CONTENT_TYPE, "application/json") - .header(header::CACHE_CONTROL, "no-cache, no-store, must-revalidate") - .body(Body::from(body_bytes)) - .unwrap() - .into_response(); + tokio::spawn(async move { + let file_path = path_service.get_root_path().join(&file_path_rel); + tracing::info!("🖼️ Generating thumbnails for: {}", file_id); + thumbnail_service.generate_all_sizes_background(file_id, file_path); + }); } - // Fallback for errors - (StatusCode::INTERNAL_SERVER_ERROR, "Upload processing error").into_response() + Self::created_json_response(&file).into_response() } /// Lists files, optionally filtered by folder ID diff --git a/src/interfaces/api/handlers/search_handler.rs b/src/interfaces/api/handlers/search_handler.rs index 876a011a..7bfbe545 100644 --- a/src/interfaces/api/handlers/search_handler.rs +++ b/src/interfaces/api/handlers/search_handler.rs @@ -8,6 +8,7 @@ use tracing::{error, info}; use crate::application::dtos::search_dto::SearchCriteriaDto; use crate::common::di::AppState; +use crate::interfaces::middleware::auth::AuthUser; use std::sync::Arc; /** @@ -23,6 +24,7 @@ impl SearchHandler { /// GET /search — simple query-parameter-based search. pub async fn search_files_get( State(state): State>, + auth_user: AuthUser, Query(params): Query, ) -> impl IntoResponse { info!("API: File search with parameters: {:?}", params); @@ -57,7 +59,7 @@ impl SearchHandler { sort_by: params.sort_by.unwrap_or_else(|| "relevance".to_string()), }; - match search_service.search(search_criteria).await { + match search_service.search(search_criteria, &auth_user.id).await { Ok(results) => { info!( "Search completed in {}ms — {} files, {} folders", @@ -81,6 +83,7 @@ impl SearchHandler { /// POST /search/advanced — full criteria in the request body. pub async fn search_files_post( State(state): State>, + auth_user: AuthUser, Json(criteria): Json, ) -> impl IntoResponse { info!("API: Advanced file search"); @@ -97,7 +100,7 @@ impl SearchHandler { } }; - match search_service.search(criteria).await { + match search_service.search(criteria, &auth_user.id).await { Ok(results) => { info!( "Advanced search completed in {}ms — {} files, {} folders",