From 661c9cb688ad3abd28bf518d4e886099c10fd3d1 Mon Sep 17 00:00:00 2001 From: Diocrafts Date: Sat, 7 Mar 2026 19:15:36 +0100 Subject: [PATCH] fix: delete thumbnails when files are permanently deleted - Inject ThumbnailService into TrashService and FileManagementService - Call delete_thumbnails() after permanent file deletion in: - TrashService::delete_permanently (single item) - TrashService::empty_trash (bulk: collects file IDs first) - FileManagementService::delete_file - FileManagementService::delete_with_cleanup (fallback path) - All thumbnail cleanup is best-effort (warn on failure, never blocks) - Prevents orphaned thumbnail files from accumulating on disk --- .../services/file_management_service.rs | 20 ++++++++- src/application/services/trash_service.rs | 44 ++++++++++++++++++- src/common/di.rs | 5 ++- 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/src/application/services/file_management_service.rs b/src/application/services/file_management_service.rs index 9a003362..06ba7ff5 100755 --- a/src/application/services/file_management_service.rs +++ b/src/application/services/file_management_service.rs @@ -8,6 +8,7 @@ use crate::application::services::trash_service::TrashService; use crate::common::errors::DomainError; use crate::infrastructure::repositories::pg::file_blob_read_repository::FileBlobReadRepository; use crate::infrastructure::repositories::pg::file_blob_write_repository::FileBlobWriteRepository; +use crate::infrastructure::services::thumbnail_service::ThumbnailService; use tracing::{error, info, warn}; use uuid::Uuid; @@ -21,6 +22,7 @@ pub struct FileManagementService { file_repository: Arc, file_read: Option>, trash_service: Option>, + thumbnail_service: Option>, } impl FileManagementService { @@ -30,6 +32,7 @@ impl FileManagementService { file_repository, file_read: None, trash_service: None, + thumbnail_service: None, } } @@ -38,11 +41,13 @@ impl FileManagementService { file_repository: Arc, trash_service: Option>, file_read: Option>, + thumbnail_service: Option>, ) -> Self { Self { file_repository, file_read, trash_service, + thumbnail_service, } } @@ -171,7 +176,14 @@ impl FileManagementUseCase for FileManagementService { } async fn delete_file(&self, id: &str) -> Result<(), DomainError> { - self.file_repository.delete_file(id).await + self.file_repository.delete_file(id).await?; + // Best-effort thumbnail cleanup + if let Some(thumb) = &self.thumbnail_service { + if let Err(e) = thumb.delete_thumbnails(id).await { + warn!("Failed to delete thumbnails for file {}: {}", id, e); + } + } + Ok(()) } async fn delete_file_owned(&self, id: &str, caller_id: Uuid) -> Result<(), DomainError> { @@ -210,6 +222,12 @@ impl FileManagementUseCase for FileManagementService { // Step 2: Permanent delete — trigger handles blob ref_count warn!("Permanently deleting file: {}", id); self.file_repository.delete_file(id).await?; + // Best-effort thumbnail cleanup + if let Some(thumb) = &self.thumbnail_service { + if let Err(e) = thumb.delete_thumbnails(id).await { + warn!("Failed to delete thumbnails for file {}: {}", id, e); + } + } info!("File permanently deleted: {}", id); Ok(false) // permanently deleted diff --git a/src/application/services/trash_service.rs b/src/application/services/trash_service.rs index 1e375ffb..881bd4e5 100755 --- a/src/application/services/trash_service.rs +++ b/src/application/services/trash_service.rs @@ -1,5 +1,5 @@ use std::sync::Arc; -use tracing::{debug, error, info, instrument}; +use tracing::{debug, error, info, instrument, warn}; use uuid::Uuid; use crate::application::dtos::trash_dto::TrashedItemDto; @@ -13,6 +13,7 @@ use crate::infrastructure::repositories::pg::file_blob_read_repository::FileBlob use crate::infrastructure::repositories::pg::file_blob_write_repository::FileBlobWriteRepository; use crate::infrastructure::repositories::pg::folder_db_repository::FolderDbRepository; use crate::infrastructure::repositories::pg::trash_db_repository::TrashDbRepository; +use crate::infrastructure::services::thumbnail_service::ThumbnailService; /** * Application service for trash operations. @@ -40,6 +41,9 @@ pub struct TrashService { /// Port for folder operations (get folder, trash, restore, delete) folder_storage_port: Arc, + /// Thumbnail service for cleaning up thumbnails on permanent delete + thumbnail_service: Option>, + /// Number of days items should be kept in trash before automatic cleanup retention_days: u32, } @@ -51,12 +55,14 @@ impl TrashService { file_write_port: Arc, folder_storage_port: Arc, retention_days: u32, + thumbnail_service: Option>, ) -> Self { Self { trash_repository, file_read_port, file_write_port, folder_storage_port, + thumbnail_service, retention_days, } } @@ -557,6 +563,14 @@ impl TrashUseCase for TrashService { } } } + + // Best-effort thumbnail cleanup — thumbnails are cache + // artifacts, so failure must not block file deletion. + if let Some(thumb) = &self.thumbnail_service { + if let Err(e) = thumb.delete_thumbnails(&file_id).await { + warn!("Failed to delete thumbnails for file {}: {}", file_id, e); + } + } } TrashedItemType::Folder => { // Permanently delete the folder @@ -647,6 +661,25 @@ impl TrashUseCase for TrashService { async fn empty_trash(&self, user_id: Uuid) -> Result<()> { info!("Emptying trash for user {}", user_id); + // Collect trashed file IDs BEFORE bulk-deleting so we can clean up + // their thumbnails afterward. This is best-effort — if the query + // fails we still proceed with the bulk delete. + let trashed_file_ids: Vec = if self.thumbnail_service.is_some() { + match self.trash_repository.get_trash_items(&user_id).await { + Ok(items) => items + .iter() + .filter(|i| matches!(i.item_type(), TrashedItemType::File)) + .map(|i| i.original_id().to_string()) + .collect(), + Err(e) => { + warn!("Could not list trashed items for thumbnail cleanup: {}", e); + Vec::new() + } + } + } else { + Vec::new() + }; + // clear_trash() already performs bulk SQL DELETEs in 2 queries: // 1. DELETE FROM storage.files WHERE user_id = $1 AND is_trashed = TRUE // 2. DELETE FROM storage.folders WHERE user_id = $1 AND is_trashed = TRUE @@ -659,6 +692,15 @@ impl TrashUseCase for TrashService { // Finally it clears the trash_items index for the user. self.trash_repository.clear_trash(&user_id).await?; + // Best-effort thumbnail cleanup for all deleted files + if let Some(thumb) = &self.thumbnail_service { + for file_id in &trashed_file_ids { + if let Err(e) = thumb.delete_thumbnails(file_id).await { + warn!("Failed to delete thumbnails for file {}: {}", file_id, e); + } + } + } + info!("Trash emptied for user {}", user_id); Ok(()) } diff --git a/src/common/di.rs b/src/common/di.rs index c89374b0..1ad614e8 100755 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -261,6 +261,7 @@ impl AppServiceFactory { repos.file_write_repository.clone(), trash_service.clone(), Some(repos.file_read_repository.clone()), + Some(core.thumbnail_service.clone()), )); let file_use_case_factory = Arc::new(AppFileUseCaseFactory::new( @@ -302,6 +303,7 @@ impl AppServiceFactory { pub async fn create_trash_service( &self, repos: &RepositoryServices, + core: &CoreServices, ) -> Option> { if !self.config.features.enable_trash { tracing::info!("Trash service is disabled in configuration"); @@ -317,6 +319,7 @@ impl AppServiceFactory { repos.file_write_repository.clone(), repos.folder_repository.clone(), self.config.storage.trash_retention_days, + Some(core.thumbnail_service.clone()), )); // Initialize cleanup service (bulk-deletes expired items in 2 SQL queries) @@ -457,7 +460,7 @@ impl AppServiceFactory { let repos = self.create_repository_services(&core, &pool); // 3. Trash service (needed before application services) - let trash_service = self.create_trash_service(&repos).await; + let trash_service = self.create_trash_service(&repos, &core).await; // 4. Application services (with trash already wired) let mut apps = self.create_application_services(&core, &repos, trash_service.clone());