From 2dde4da5cf1fe4a7f09c6eaf923f30e97ccbfb2e Mon Sep 17 00:00:00 2001 From: Diocrafts Date: Sun, 12 Apr 2026 00:50:10 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20thumbnail=20dedup=20=E2=80=94=20store?= =?UTF-8?q?=20thumbnails=20by=20blob=5Fhash=20instead=20of=20file=5Fid=20(?= =?UTF-8?q?#233)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thumbnails are now keyed by blob_hash on disk so identical files share a single set of thumbnails (icon/preview/large). For 4000 duplicate files with the same content, this reduces thumbnail storage from 12,000 files to just 3. Changes: - get_thumbnail_path() keys by blob_hash instead of file_id - get_thumbnail(), get_cached_thumbnail(), generate_all_sizes_background() accept blob_hash parameter for disk dedup - generate_all_sizes_background() fast path: if blob-hash thumbnails already exist on disk, skip image processing entirely and just populate moka cache for the new file_id - delete_thumbnails() only invalidates moka cache (shared disk thumbnails must not be deleted when one file is removed) - delete_blob_thumbnails() added for GC; garbage_collect() now cleans up orphaned thumbnail files alongside blob files - External thumbnails (video frames) stored as ext-{file_id}.jpg since they are client-generated and not dedup-able - ThumbnailPort trait updated with blob_hash parameters - All handler call sites updated (file_handler, preview_handler) - Tests updated for new signatures --- src/application/ports/thumbnail_ports.rs | 26 ++- src/infrastructure/services/dedup_service.rs | 9 + .../services/thumbnail_service.rs | 157 ++++++++++++++---- .../services/thumbnail_service_test.rs | 4 +- src/interfaces/api/handlers/file_handler.rs | 7 +- src/interfaces/nextcloud/preview_handler.rs | 2 +- 6 files changed, 164 insertions(+), 41 deletions(-) diff --git a/src/application/ports/thumbnail_ports.rs b/src/application/ports/thumbnail_ports.rs index 5d5b5fa7..4eb31f8e 100644 --- a/src/application/ports/thumbnail_ports.rs +++ b/src/application/ports/thumbnail_ports.rs @@ -68,18 +68,27 @@ pub trait ThumbnailPort: Send + Sync + 'static { /// Get a thumbnail, generating it on-demand if needed. /// - /// Returns the thumbnail bytes in WebP format. + /// `blob_hash` is the content hash used as the disk storage key + /// (dedup: identical blobs share one set of thumbnails). async fn get_thumbnail( &self, file_id: &str, + blob_hash: &str, size: ThumbnailSize, original_path: &Path, ) -> Result; /// Generate all thumbnail sizes for a file in the background. /// - /// Called after file upload to pre-generate thumbnails. - fn generate_all_sizes_background(self: Arc, file_id: String, original_path: PathBuf); + /// `blob_hash` is the content hash used as the disk storage key. + /// If thumbnails already exist for this hash, only the moka cache + /// is populated (zero CPU for image processing). + fn generate_all_sizes_background( + self: Arc, + file_id: String, + blob_hash: String, + original_path: PathBuf, + ); /// Delete all thumbnails for a file. async fn delete_thumbnails(&self, file_id: &str) -> Result<(), DomainError>; @@ -87,9 +96,14 @@ pub trait ThumbnailPort: Send + Sync + 'static { /// Try to get a cached thumbnail without generating one. /// /// Returns `None` if no cached thumbnail exists on disk or in memory. - /// Used for non-image file types (videos) where thumbnails are - /// generated client-side and uploaded. - async fn get_cached_thumbnail(&self, file_id: &str, size: ThumbnailSize) -> Option; + /// `blob_hash` is used to locate the file on disk. If `None`, only + /// the in-memory moka cache is checked. + async fn get_cached_thumbnail( + &self, + file_id: &str, + blob_hash: Option<&str>, + size: ThumbnailSize, + ) -> Option; /// Store an externally-generated thumbnail (e.g. client-side video frame). /// diff --git a/src/infrastructure/services/dedup_service.rs b/src/infrastructure/services/dedup_service.rs index 587b985c..a60ce7f8 100644 --- a/src/infrastructure/services/dedup_service.rs +++ b/src/infrastructure/services/dedup_service.rs @@ -714,11 +714,20 @@ impl DedupService { } // Delete blob files OUTSIDE the TX (already committed). + // Also clean up any thumbnail files for these blob hashes + // (thumbnails are keyed by blob_hash and live under + // storage_root/.thumbnails/{icon,preview,large}/{hash}.jpg). + let thumbnails_root = self.blob_root.parent().unwrap_or(&self.blob_root).join(".thumbnails"); for (hash, size) in &batch { let blob_path = self.blob_path(hash); if let Err(e) = fs::remove_file(&blob_path).await { tracing::warn!("Failed to delete orphan blob file {hash}: {e}"); } + // Remove associated thumbnail files (best-effort) + for dir in &["icon", "preview", "large"] { + let thumb = thumbnails_root.join(dir).join(format!("{hash}.jpg")); + let _ = fs::remove_file(&thumb).await; + } total_bytes += *size as u64; } total_deleted += batch.len() as u64; diff --git a/src/infrastructure/services/thumbnail_service.rs b/src/infrastructure/services/thumbnail_service.rs index 1b38fd7f..eca84729 100644 --- a/src/infrastructure/services/thumbnail_service.rs +++ b/src/infrastructure/services/thumbnail_service.rs @@ -168,17 +168,18 @@ impl ThumbnailService { ) } - /// Get the path where a thumbnail would be stored - fn get_thumbnail_path(&self, file_id: &str, size: ThumbnailSize) -> PathBuf { + /// Get the path where a thumbnail would be stored (keyed by blob hash for dedup). + fn get_thumbnail_path(&self, blob_hash: &str, size: ThumbnailSize) -> PathBuf { self.thumbnails_root .join(size.dir_name()) - .join(format!("{}.jpg", file_id)) + .join(format!("{}.jpg", blob_hash)) } /// Get a thumbnail, generating it if needed. /// /// # Arguments - /// * `file_id` - ID of the original file + /// * `file_id` - ID of the original file (used as moka cache key) + /// * `blob_hash` - Content hash of the file (used as disk key for dedup) /// * `size` - Desired thumbnail size /// * `original_path` - Path to the original image file /// @@ -187,6 +188,7 @@ impl ThumbnailService { pub async fn get_thumbnail( &self, file_id: &str, + blob_hash: &str, size: ThumbnailSize, original_path: &Path, ) -> Result { @@ -195,7 +197,7 @@ impl ThumbnailService { size, }; - let thumb_path = self.get_thumbnail_path(file_id, size); + let thumb_path = self.get_thumbnail_path(blob_hash, size); let original_owned = original_path.to_path_buf(); let file_id_owned = file_id.to_string(); @@ -257,7 +259,16 @@ impl ThumbnailService { /// Unlike `get_thumbnail`, this does **not** generate a new thumbnail. /// Useful for non-image file types (videos) where a client-generated /// thumbnail may have been uploaded previously. - pub async fn get_cached_thumbnail(&self, file_id: &str, size: ThumbnailSize) -> Option { + /// + /// `blob_hash` is used to locate the file on disk (dedup-aware). + /// If `None`, only the in-memory cache is checked (used for video + /// thumbnails where blob_hash is not yet resolved). + pub async fn get_cached_thumbnail( + &self, + file_id: &str, + blob_hash: Option<&str>, + size: ThumbnailSize, + ) -> Option { // 1. Check in-memory cache let cache_key = ThumbnailCacheKey { file_id: file_id.to_string(), @@ -269,8 +280,9 @@ impl ThumbnailService { return Some(bytes); } - // 2. Check disk - let thumb_path = self.get_thumbnail_path(file_id, size); + // 2. Check disk (needs blob_hash to locate the shared file) + let hash = blob_hash?; + let thumb_path = self.get_thumbnail_path(hash, size); if let Ok(data) = fs::read(&thumb_path).await { let bytes = Bytes::from(data); // Populate in-memory cache for next hit @@ -290,6 +302,9 @@ impl ThumbnailService { /// /// **Slow path**: decode → optional resize → re-encode to JPEG q=80. /// Only triggered when a client sends an oversized or non-JPEG image. + /// + /// External thumbnails (video frames) are stored by `file_id` since + /// they are client-generated and not dedup-able. pub async fn store_external_thumbnail( &self, file_id: &str, @@ -345,8 +360,11 @@ impl ThumbnailService { let bytes = Bytes::from(jpeg_bytes); - // Save to disk - let thumb_path = self.get_thumbnail_path(file_id, size); + // External thumbnails are stored by file_id (not dedup-able) + let thumb_path = self + .thumbnails_root + .join(size.dir_name()) + .join(format!("ext-{}.jpg", file_id)); if let Some(parent) = thumb_path.parent() { let _ = fs::create_dir_all(parent).await; } @@ -472,16 +490,60 @@ impl ThumbnailService { /// Generate all thumbnail sizes for a file in the background. /// + /// Thumbnails are stored on disk keyed by `blob_hash`, so duplicate + /// uploads with the same content share a single set of thumbnails. + /// If thumbnails already exist for this `blob_hash`, only the moka + /// cache is populated (zero CPU for image processing). + /// /// Loads the image **once** and produces all 3 sizes (Icon, Preview, /// Large) inside a single `spawn_blocking` call. This avoids 3× /// I/O reads and 3× JPEG/PNG decode — reducing CPU time by ~45% /// and peak RAM from ~540 MB to ~180 MB for concurrent uploads. /// The encoded image buffer is explicitly dropped after decoding /// to further reduce peak memory by the size of the original file. - pub fn generate_all_sizes_background(self: Arc, file_id: String, original_path: PathBuf) { + pub fn generate_all_sizes_background( + self: Arc, + file_id: String, + blob_hash: String, + original_path: PathBuf, + ) { tokio::spawn(async move { tracing::info!("🖼️ Background thumbnail generation starting: {}", file_id); + // ── Fast path: blob-hash thumbnails already exist on disk ──── + // If another file with the same content was already uploaded, + // the thumbnails exist. Just populate the moka cache for this + // file_id and skip image processing entirely. + let all_exist = { + let mut ok = true; + for size in ThumbnailSize::all() { + let thumb_path = self.get_thumbnail_path(&blob_hash, *size); + if fs::metadata(&thumb_path).await.is_err() { + ok = false; + break; + } + } + ok + }; + if all_exist { + for size in ThumbnailSize::all() { + let thumb_path = self.get_thumbnail_path(&blob_hash, *size); + if let Ok(data) = fs::read(&thumb_path).await { + let cache_key = ThumbnailCacheKey { + file_id: file_id.clone(), + size: *size, + }; + self.cache.insert(cache_key, Bytes::from(data)).await; + } + } + tracing::info!( + "🖼️ Thumbnail dedup hit for {} (blob {}): skipped generation", + file_id, + &blob_hash[..12] + ); + return; + } + // Acquire semaphore permit — bounds peak RAM from concurrent decodes let _permit = match self.decode_semaphore.acquire().await { Ok(p) => p, @@ -579,10 +641,10 @@ impl ThumbnailService { } }; - // Save each size to disk AND populate moka so the very first - // GET after upload is served from RAM (zero disk I/O). + // Save each size to disk (keyed by blob_hash for dedup) + // AND populate moka (keyed by file_id for fast serving). for (size, bytes) in thumbnails { - let thumb_path = self.get_thumbnail_path(&file_id, size); + let thumb_path = self.get_thumbnail_path(&blob_hash, size); if let Some(parent) = thumb_path.parent() { let _ = fs::create_dir_all(parent).await; } @@ -603,28 +665,53 @@ impl ThumbnailService { }); } - /// Delete all thumbnails for a file + /// Delete thumbnails for a file. + /// + /// Only invalidates the in-memory moka cache (keyed by file_id). + /// Disk thumbnails are keyed by blob_hash and may be shared by + /// other files with the same content — they are cleaned up via + /// `delete_blob_thumbnails` when the blob is garbage-collected. + /// Also removes any external (video-frame) thumbnails stored by file_id. pub async fn delete_thumbnails(&self, file_id: &str) -> Result<(), ThumbnailError> { for size in ThumbnailSize::all() { - let path = self.get_thumbnail_path(file_id, *size); - if fs::metadata(&path).await.is_ok() { - fs::remove_file(&path) - .await - .map_err(|e| ThumbnailError::IoError(e.to_string()))?; - } - - // Remove from cache (lock-free invalidation) + // Remove from moka cache (lock-free invalidation) let cache_key = ThumbnailCacheKey { file_id: file_id.to_string(), size: *size, }; self.cache.invalidate(&cache_key).await; + + // Remove external (video-frame) thumbnails stored by file_id + let ext_path = self + .thumbnails_root + .join(size.dir_name()) + .join(format!("ext-{}.jpg", file_id)); + if fs::metadata(&ext_path).await.is_ok() { + let _ = fs::remove_file(&ext_path).await; + } } - tracing::debug!("🗑️ Deleted thumbnails for: {}", file_id); + tracing::debug!("🗑️ Invalidated thumbnail cache for: {}", file_id); Ok(()) } + /// Remove orphaned blob-hash thumbnails whose blob no longer exists. + /// + /// Call during blob garbage collection: pass the hash of the blob + /// being deleted and the corresponding thumbnails are removed from disk. + pub async fn delete_blob_thumbnails(&self, blob_hash: &str) { + for size in ThumbnailSize::all() { + let path = self.get_thumbnail_path(blob_hash, *size); + if fs::metadata(&path).await.is_ok() { + let _ = fs::remove_file(&path).await; + } + } + tracing::debug!( + "🗑️ Deleted blob thumbnails for hash: {}…", + &blob_hash[..blob_hash.len().min(12)] + ); + } + /// Get cache statistics pub async fn get_stats(&self) -> ThumbnailStats { ThumbnailStats { @@ -656,16 +743,22 @@ impl ThumbnailPort for ThumbnailService { async fn get_thumbnail( &self, file_id: &str, + blob_hash: &str, size: PortThumbnailSize, original_path: &Path, ) -> Result { - self.get_thumbnail(file_id, size.into(), original_path) + self.get_thumbnail(file_id, blob_hash, size.into(), original_path) .await .map_err(|e| DomainError::new(ErrorKind::InternalError, "Thumbnail", e.to_string())) } - fn generate_all_sizes_background(self: Arc, file_id: String, original_path: PathBuf) { - ThumbnailService::generate_all_sizes_background(self, file_id, original_path) + fn generate_all_sizes_background( + self: Arc, + file_id: String, + blob_hash: String, + original_path: PathBuf, + ) { + ThumbnailService::generate_all_sizes_background(self, file_id, blob_hash, original_path) } async fn delete_thumbnails(&self, file_id: &str) -> Result<(), DomainError> { @@ -674,8 +767,14 @@ impl ThumbnailPort for ThumbnailService { .map_err(|e| DomainError::new(ErrorKind::InternalError, "Thumbnail", e.to_string())) } - async fn get_cached_thumbnail(&self, file_id: &str, size: PortThumbnailSize) -> Option { - self.get_cached_thumbnail(file_id, size.into()).await + async fn get_cached_thumbnail( + &self, + file_id: &str, + blob_hash: Option<&str>, + size: PortThumbnailSize, + ) -> Option { + self.get_cached_thumbnail(file_id, blob_hash, size.into()) + .await } async fn store_external_thumbnail( diff --git a/src/infrastructure/services/thumbnail_service_test.rs b/src/infrastructure/services/thumbnail_service_test.rs index 37d50c56..807fdd41 100644 --- a/src/infrastructure/services/thumbnail_service_test.rs +++ b/src/infrastructure/services/thumbnail_service_test.rs @@ -40,7 +40,7 @@ async fn generate_thumbnail_from_blob_path() { // The key assertion: the service can read from a blob path (not a logical path) let result = svc - .get_thumbnail("test-file-id", ThumbnailSize::Icon, &blob_path) + .get_thumbnail("test-file-id", "ab1234567890", ThumbnailSize::Icon, &blob_path) .await; let thumb_bytes = result.expect("thumbnail generation should succeed from blob path"); @@ -67,7 +67,7 @@ async fn generate_thumbnail_nonexistent_path_returns_error() { let bad_path = tmp.path().join("does-not-exist.png"); let result = svc - .get_thumbnail("missing-id", ThumbnailSize::Icon, &bad_path) + .get_thumbnail("missing-id", "nonexistent-hash", ThumbnailSize::Icon, &bad_path) .await; assert!(result.is_err(), "should fail for nonexistent file"); diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index 67810447..6686a464 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -359,7 +359,7 @@ impl FileHandler { // Try moka (RAM) → disk before touching the database. // If the thumbnail exists it was authorized at creation time. if let Some(data) = thumbnail_service - .get_cached_thumbnail(&id, thumb_size.into()) + .get_cached_thumbnail(&id, None, thumb_size.into()) .await { return Response::builder() @@ -411,7 +411,7 @@ impl FileHandler { let file_path = state.core.dedup_service.blob_path(&blob_hash); match thumbnail_service - .get_thumbnail(&id, thumb_size.into(), &file_path) + .get_thumbnail(&id, &blob_hash, thumb_size.into(), &file_path) .await { Ok(data) => Response::builder() @@ -742,10 +742,11 @@ impl FileHandler { let file_id = file.id.clone(); let thumbnail_service = state.core.thumbnail_service.clone(); let file_path = state.core.dedup_service.blob_path(&blob_hash); + let blob_hash_owned = blob_hash.clone(); tokio::spawn(async move { tracing::info!("🖼️ Generating thumbnails for: {}", file_id); - thumbnail_service.generate_all_sizes_background(file_id, file_path); + thumbnail_service.generate_all_sizes_background(file_id, blob_hash_owned, file_path); }); } diff --git a/src/interfaces/nextcloud/preview_handler.rs b/src/interfaces/nextcloud/preview_handler.rs index e5f4ae6b..3b177802 100644 --- a/src/interfaces/nextcloud/preview_handler.rs +++ b/src/interfaces/nextcloud/preview_handler.rs @@ -146,7 +146,7 @@ pub async fn handle_preview( match state .core .thumbnail_service - .get_thumbnail(&object_id, thumb_size.into(), &blob_path) + .get_thumbnail(&object_id, &blob_hash, thumb_size.into(), &blob_path) .await { Ok(data) => {