From 30fd3cc48872d0f981354d07aa97a225cad9720f Mon Sep 17 00:00:00 2001 From: DioCrafts Date: Sun, 26 Apr 2026 11:55:05 +0200 Subject: [PATCH] Fix thumbnail generation for CDC blob storage --- .../services/auth_application_service.rs | 4 +- src/infrastructure/services/dedup_service.rs | 20 + .../services/thumbnail_service.rs | 359 +++++++++++++++--- .../services/thumbnail_service_test.rs | 34 ++ src/interfaces/api/handlers/file_handler.rs | 56 ++- src/interfaces/nextcloud/preview_handler.rs | 38 +- 6 files changed, 435 insertions(+), 76 deletions(-) diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 9dace7c3..8aaf8687 100644 --- a/src/application/services/auth_application_service.rs +++ b/src/application/services/auth_application_service.rs @@ -1162,7 +1162,9 @@ impl AuthApplicationService { // Filter helper: removes any chars that are not valid in a username let filter_username_chars = |s: &str| { s.chars() - .filter(|c| c.is_ascii_alphanumeric() || *c == '-' || *c == '_' || *c == '.') + .filter(|c| { + c.is_ascii_alphanumeric() || *c == '-' || *c == '_' || *c == '.' + }) .take(32) .collect::() }; diff --git a/src/infrastructure/services/dedup_service.rs b/src/infrastructure/services/dedup_service.rs index 9d1d4903..77e49551 100644 --- a/src/infrastructure/services/dedup_service.rs +++ b/src/infrastructure/services/dedup_service.rs @@ -918,6 +918,26 @@ impl DedupService { } } + /// Read the full blob into memory — CDC-aware with legacy fallback. + /// + /// This is intended for image-oriented workflows such as thumbnail + /// generation where the downstream library already requires the full + /// payload in memory to decode the image. + pub async fn read_blob_bytes(&self, hash: &str) -> Result { + let expected_size = self.blob_size(hash).await? as usize; + let mut data = Vec::with_capacity(expected_size); + let mut stream = self.read_blob_stream(hash).await?; + + while let Some(chunk) = stream.next().await { + let chunk = chunk.map_err(|e| { + DomainError::internal_error("Dedup", format!("Failed to read blob chunk: {}", e)) + })?; + data.extend_from_slice(&chunk); + } + + Ok(Bytes::from(data)) + } + /// Stream a byte range — CDC-aware with legacy fallback. /// /// For CDC files: calculates which chunks overlap the requested range, diff --git a/src/infrastructure/services/thumbnail_service.rs b/src/infrastructure/services/thumbnail_service.rs index bd65e442..7c3622f1 100644 --- a/src/infrastructure/services/thumbnail_service.rs +++ b/src/infrastructure/services/thumbnail_service.rs @@ -254,6 +254,76 @@ impl ThumbnailService { Ok(bytes) } + /// Get a thumbnail from raw image bytes, generating it if needed. + /// + /// This is the storage-model-safe entrypoint for CDC/manifest-backed + /// blobs where no single local source file exists on disk. + pub async fn get_thumbnail_from_bytes( + &self, + file_id: &str, + blob_hash: &str, + size: ThumbnailSize, + original_data: Bytes, + ) -> Result { + let cache_key = ThumbnailCacheKey { + file_id: file_id.to_string(), + size, + }; + + let thumb_path = self.get_thumbnail_path(blob_hash, size); + let file_id_owned = file_id.to_string(); + + let entry = self + .cache + .entry(cache_key) + .or_insert_with(async move { + if let Ok(data) = fs::read(&thumb_path).await { + tracing::debug!( + "💾 Thumbnail loaded from disk: {} {:?}", + file_id_owned, + size + ); + return Bytes::from(data); + } + + tracing::info!("🎨 Generating thumbnail: {} {:?}", file_id_owned, size); + match Self::generate_thumbnail_from_data( + original_data, + size, + self.generation_timeout, + ) + .await + { + Ok(bytes) => { + if let Some(parent) = thumb_path.parent() { + let _ = fs::create_dir_all(parent).await; + } + let _ = fs::write(&thumb_path, &bytes).await; + bytes + } + Err(e) => { + tracing::warn!( + "Thumbnail generation failed for {} {:?}: {e}", + file_id_owned, + size + ); + Bytes::new() + } + } + }) + .await; + + let bytes = entry.into_value(); + if bytes.is_empty() { + return Err(ThumbnailError::ImageError( + "Thumbnail generation failed".to_string(), + )); + } + + tracing::debug!("🔥 Thumbnail served: {} {:?}", file_id, size); + Ok(bytes) + } + /// Try to serve a thumbnail from cache only (memory → disk). /// /// Unlike `get_thumbnail`, this does **not** generate a new thumbnail. @@ -395,6 +465,141 @@ impl ThumbnailService { Ok(bytes) } + fn render_thumbnail_from_data( + data: &[u8], + size: ThumbnailSize, + ) -> Result, ThumbnailError> { + let max_dim = size.max_dimension(); + + let (w, h) = image::ImageReader::new(std::io::Cursor::new(data)) + .with_guessed_format() + .map_err(|e| ThumbnailError::ImageError(e.to_string()))? + .into_dimensions() + .map_err(|e| ThumbnailError::ImageError(e.to_string()))?; + if (w as u64) * (h as u64) > MAX_DECODE_PIXELS { + return Err(ThumbnailError::ImageError(format!( + "Image too large for thumbnail: {w}×{h} ({} MP, max {MAX_DECODE_PIXELS})", + w as u64 * h as u64 / 1_000_000 + ))); + } + + let img = + image::load_from_memory(data).map_err(|e| ThumbnailError::ImageError(e.to_string()))?; + + let img = { + use crate::infrastructure::services::exif_service::{ExifService, apply_orientation}; + let orientation = ExifService::extract(data) + .and_then(|m| m.orientation) + .unwrap_or(1); + apply_orientation(img, orientation) + }; + + let (orig_width, orig_height) = (img.width(), img.height()); + let (new_width, new_height) = if orig_width > orig_height { + let ratio = max_dim as f32 / orig_width as f32; + (max_dim, (orig_height as f32 * ratio) as u32) + } else { + let ratio = max_dim as f32 / orig_height as f32; + ((orig_width as f32 * ratio) as u32, max_dim) + }; + + let filter = match size { + ThumbnailSize::Icon => FilterType::Triangle, + ThumbnailSize::Preview => FilterType::CatmullRom, + ThumbnailSize::Large => FilterType::CatmullRom, + }; + let thumbnail = img.resize(new_width, new_height, filter); + + let rgb = thumbnail.to_rgb8(); + let mut buffer = Vec::new(); + let encoder = JpegEncoder::new_with_quality(&mut buffer, 80); + rgb.write_with_encoder(encoder) + .map_err(|e| ThumbnailError::ImageError(e.to_string()))?; + + Ok(buffer) + } + + fn render_all_thumbnails_from_data( + data: &[u8], + ) -> Result, ThumbnailError> { + let (w, h) = image::ImageReader::new(std::io::Cursor::new(data)) + .with_guessed_format() + .map_err(|e| ThumbnailError::ImageError(e.to_string()))? + .into_dimensions() + .map_err(|e| ThumbnailError::ImageError(e.to_string()))?; + if (w as u64) * (h as u64) > MAX_DECODE_PIXELS { + return Err(ThumbnailError::ImageError(format!( + "Image too large for thumbnail: {w}×{h} ({} MP, max {MAX_DECODE_PIXELS})", + w as u64 * h as u64 / 1_000_000 + ))); + } + + let img = + image::load_from_memory(data).map_err(|e| ThumbnailError::ImageError(e.to_string()))?; + + let img = { + use crate::infrastructure::services::exif_service::{ExifService, apply_orientation}; + let orientation = ExifService::extract(data) + .and_then(|m| m.orientation) + .unwrap_or(1); + apply_orientation(img, orientation) + }; + + let (orig_w, orig_h) = (img.width(), img.height()); + + ThumbnailSize::all() + .par_iter() + .map(|&size| { + let max_dim = size.max_dimension(); + + let (new_w, new_h) = if orig_w > orig_h { + let ratio = max_dim as f32 / orig_w as f32; + (max_dim, (orig_h as f32 * ratio) as u32) + } else { + let ratio = max_dim as f32 / orig_h as f32; + ((orig_w as f32 * ratio) as u32, max_dim) + }; + + let filter = match size { + ThumbnailSize::Icon => FilterType::Triangle, + ThumbnailSize::Preview => FilterType::CatmullRom, + ThumbnailSize::Large => FilterType::CatmullRom, + }; + let thumb = img.resize(new_w, new_h, filter); + + let rgb = thumb.to_rgb8(); + let mut buf = Vec::new(); + let encoder = JpegEncoder::new_with_quality(&mut buf, 80); + rgb.write_with_encoder(encoder) + .map_err(|e| ThumbnailError::ImageError(e.to_string()))?; + + Ok((size, Bytes::from(buf))) + }) + .collect::, ThumbnailError>>() + } + + async fn generate_thumbnail_from_data( + original_data: Bytes, + size: ThumbnailSize, + timeout_duration: Duration, + ) -> Result { + let spawn_result = tokio::task::spawn_blocking(move || { + Self::render_thumbnail_from_data(original_data.as_ref(), size) + }); + + let result = timeout(timeout_duration, spawn_result) + .await + .map_err(|_| { + ThumbnailError::TaskError(format!( + "Thumbnail generation timed out after {:?}", + timeout_duration + )) + })? + .map_err(|e| ThumbnailError::TaskError(e.to_string()))?; + + result.map(Bytes::from) + } + /// Generate a thumbnail from an image file. /// /// Concurrency is bounded by `decode_semaphore` to prevent OOM when @@ -409,7 +614,6 @@ impl ThumbnailService { size: ThumbnailSize, ) -> Result { let path = original_path.to_path_buf(); - let max_dim = size.max_dimension(); let timeout_duration = self.generation_timeout; // Acquire semaphore permit — bounds peak RAM from concurrent decodes @@ -422,68 +626,9 @@ impl ThumbnailService { // Run image processing in blocking thread pool with timeout let spawn_result = tokio::task::spawn_blocking(move || -> Result, ThumbnailError> { - // Single read: load file once into memory, then work from the buffer let data = std::fs::read(&path).map_err(|e| ThumbnailError::ImageError(e.to_string()))?; - - // Safety check: read dimensions from in-memory buffer (no 2nd I/O) - let (w, h) = image::ImageReader::new(std::io::Cursor::new(&data)) - .with_guessed_format() - .map_err(|e| ThumbnailError::ImageError(e.to_string()))? - .into_dimensions() - .map_err(|e| ThumbnailError::ImageError(e.to_string()))?; - if (w as u64) * (h as u64) > MAX_DECODE_PIXELS { - return Err(ThumbnailError::ImageError(format!( - "Image too large for thumbnail: {w}×{h} ({} MP, max {MAX_DECODE_PIXELS})", - w as u64 * h as u64 / 1_000_000 - ))); - } - - // Full decode from the same in-memory buffer (no 2nd disk read) - let img = image::load_from_memory(&data) - .map_err(|e| ThumbnailError::ImageError(e.to_string()))?; - - // Apply EXIF orientation so thumbnails display correctly - let img = { - use crate::infrastructure::services::exif_service::{ - ExifService, apply_orientation, - }; - let orientation = ExifService::extract(&data) - .and_then(|m| m.orientation) - .unwrap_or(1); - // Free the encoded image data now that image is decoded and EXIF extracted - drop(data); - apply_orientation(img, orientation) - }; - - // Calculate new dimensions preserving aspect ratio - let (orig_width, orig_height) = (img.width(), img.height()); - let (new_width, new_height) = if orig_width > orig_height { - let ratio = max_dim as f32 / orig_width as f32; - (max_dim, (orig_height as f32 * ratio) as u32) - } else { - let ratio = max_dim as f32 / orig_height as f32; - ((orig_width as f32 * ratio) as u32, max_dim) - }; - - // Adaptive filter: faster filters for smaller sizes where - // quality difference vs Lanczos3 is imperceptible - let filter = match size { - ThumbnailSize::Icon => FilterType::Triangle, // 150px — max speed - ThumbnailSize::Preview => FilterType::CatmullRom, // 400px — good balance - ThumbnailSize::Large => FilterType::CatmullRom, // 800px — sufficient quality - }; - let thumbnail = img.resize(new_width, new_height, filter); - - // Encode as JPEG (lossy q=80) — explicit quality control, - // ~2× smaller than image-webp's Rust encoder at same visual quality - let rgb = thumbnail.to_rgb8(); - let mut buffer = Vec::new(); - let encoder = JpegEncoder::new_with_quality(&mut buffer, 80); - rgb.write_with_encoder(encoder) - .map_err(|e| ThumbnailError::ImageError(e.to_string()))?; - - Ok(buffer) + Self::render_thumbnail_from_data(&data, size) }); // Apply timeout to prevent hanging on large images @@ -677,6 +822,98 @@ impl ThumbnailService { }); } + /// Generate all thumbnail sizes in the background from raw image bytes. + /// + /// This is compatible with CDC/manifest-backed blobs because it does not + /// require a single physical source file on disk. + pub fn generate_all_sizes_background_from_bytes( + self: Arc, + file_id: String, + blob_hash: String, + original_data: Bytes, + ) { + tokio::spawn(async move { + tracing::info!("🖼️ Background thumbnail generation starting: {}", file_id); + + 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; + } + + let _permit = match self.decode_semaphore.acquire().await { + Ok(p) => p, + Err(_) => { + tracing::warn!( + "Decode semaphore closed, skipping thumbnails for {}", + file_id + ); + return; + } + }; + + let results = tokio::task::spawn_blocking(move || { + Self::render_all_thumbnails_from_data(original_data.as_ref()) + }) + .await; + + let thumbnails = match results { + Ok(Ok(t)) => t, + Ok(Err(e)) => { + tracing::warn!("Thumbnail generation failed for {}: {}", file_id, e); + return; + } + Err(e) => { + tracing::warn!("Thumbnail task panicked for {}: {}", file_id, e); + return; + } + }; + + for (size, bytes) in thumbnails { + let thumb_path = self.get_thumbnail_path(&blob_hash, size); + if let Some(parent) = thumb_path.parent() { + let _ = fs::create_dir_all(parent).await; + } + if let Err(e) = fs::write(&thumb_path, &bytes).await { + tracing::warn!("Failed to save thumbnail {} {:?}: {}", file_id, size, e); + } else { + let cache_key = ThumbnailCacheKey { + file_id: file_id.clone(), + size, + }; + self.cache.insert(cache_key, bytes).await; + tracing::debug!("✅ Generated thumbnail: {} {:?}", file_id, size); + } + } + + tracing::info!("✅ Background thumbnail generation complete: {}", file_id); + }); + } + /// Delete thumbnails for a file. /// /// Only invalidates the in-memory moka cache (keyed by file_id). diff --git a/src/infrastructure/services/thumbnail_service_test.rs b/src/infrastructure/services/thumbnail_service_test.rs index adaf31db..ac571d63 100644 --- a/src/infrastructure/services/thumbnail_service_test.rs +++ b/src/infrastructure/services/thumbnail_service_test.rs @@ -1,6 +1,8 @@ use std::sync::Arc; use std::time::Duration; +use bytes::Bytes; + use super::thumbnail_service::{ThumbnailService, ThumbnailSize}; /// Minimal valid 1x1 red PNG (68 bytes). @@ -82,3 +84,35 @@ async fn generate_thumbnail_nonexistent_path_returns_error() { assert!(result.is_err(), "should fail for nonexistent file"); } + +/// Regression test: thumbnail generation must also work when the image source +/// is reconstructed from blob storage bytes instead of a single local file. +#[tokio::test] +async fn generate_thumbnail_from_blob_bytes() { + let tmp = tempfile::tempdir().expect("create temp dir"); + let storage_root = tmp.path(); + + let svc = Arc::new(ThumbnailService::new( + storage_root, + 100, + 10 * 1024 * 1024, + Some(Duration::from_secs(30)), + )); + svc.initialize().await.expect("init thumbnail dirs"); + + let result = svc + .get_thumbnail_from_bytes( + "bytes-file-id", + "bytes-hash-123", + ThumbnailSize::Preview, + Bytes::from(tiny_png()), + ) + .await; + + let thumb_bytes = result.expect("thumbnail generation should succeed from raw bytes"); + assert!(!thumb_bytes.is_empty(), "thumbnail bytes must not be empty"); + assert!( + thumb_bytes.len() > 2 && thumb_bytes[0] == 0xFF && thumb_bytes[1] == 0xD8, + "output should be JPEG format" + ); +} diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index 599b3d1d..08ca33fc 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -396,7 +396,7 @@ impl FileHandler { .into_response(); } - // Resolve the physical blob path (content-addressable storage) + // Resolve the blob hash (content-addressable storage). let blob_hash = match state .repositories .file_read_repository @@ -408,10 +408,34 @@ impl FileHandler { return AppError::internal_error("File blob not found").into_response(); } }; - let file_path = state.core.dedup_service.blob_path(&blob_hash); + if let Some(data) = thumbnail_service + .get_cached_thumbnail(&id, Some(&blob_hash), thumb_size.into()) + .await + { + return Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "image/jpeg") + .header(header::CONTENT_LENGTH, data.len()) + .header(header::CACHE_CONTROL, "public, max-age=31536000, immutable") + .header(header::ETAG, &etag) + .body(Body::from(data)) + .unwrap() + .into_response(); + } + + let original_bytes = match state.core.dedup_service.read_blob_bytes(&blob_hash).await { + Ok(bytes) => bytes, + Err(err) => { + return AppError::internal_error(format!( + "Failed to load source image for thumbnail generation: {}", + err + )) + .into_response(); + } + }; match thumbnail_service - .get_thumbnail(&id, &blob_hash, thumb_size.into(), &file_path) + .get_thumbnail_from_bytes(&id, &blob_hash, thumb_size.into(), original_bytes) .await { Ok(data) => Response::builder() @@ -733,7 +757,8 @@ impl FileHandler { // Generate thumbnails for supported images in background. // The blob_hash was already computed during the hash-on-write spool, - // so we resolve the physical path directly — no extra DB round-trip. + // so we can reconstruct the source bytes directly from DedupService + // without an extra DB round-trip. if state .core .thumbnail_service @@ -741,16 +766,27 @@ 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 dedup_service = state.core.dedup_service.clone(); 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, - blob_hash_owned, - file_path, - ); + match dedup_service.read_blob_bytes(&blob_hash_owned).await { + Ok(original_bytes) => { + thumbnail_service.generate_all_sizes_background_from_bytes( + file_id, + blob_hash_owned, + original_bytes, + ); + } + Err(err) => { + tracing::warn!( + "Failed to load source image for thumbnail generation {}: {}", + file_id, + err + ); + } + } }); } diff --git a/src/interfaces/nextcloud/preview_handler.rs b/src/interfaces/nextcloud/preview_handler.rs index 3b177802..82028caf 100644 --- a/src/interfaces/nextcloud/preview_handler.rs +++ b/src/interfaces/nextcloud/preview_handler.rs @@ -125,7 +125,7 @@ pub async fn handle_preview( .unwrap(); } - // Get the physical blob path (content-addressable storage) + // Resolve the blob hash (content-addressable storage) let blob_hash = match state .repositories .file_read_repository @@ -140,20 +140,50 @@ pub async fn handle_preview( .unwrap(); } }; - let blob_path = state.core.dedup_service.blob_path(&blob_hash); + if let Some(data) = state + .core + .thumbnail_service + .get_cached_thumbnail(&object_id, Some(&blob_hash), thumb_size.into()) + .await + { + let etag = format!("\"thumb-{}-{:?}\"", object_id, thumb_size); + return Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "image/jpeg") + .header(header::CONTENT_LENGTH, data.len()) + .header(header::CACHE_CONTROL, "public, max-age=31536000, immutable") + .header(header::ETAG, etag) + .body(Body::from(data)) + .unwrap(); + } + + let original_bytes = match state.core.dedup_service.read_blob_bytes(&blob_hash).await { + Ok(bytes) => bytes, + Err(err) => { + tracing::error!( + "Failed to load source image for preview {}: {}", + object_id, + err + ); + return Response::builder() + .status(StatusCode::INTERNAL_SERVER_ERROR) + .body(Body::from("Failed to load preview source")) + .unwrap(); + } + }; // Generate/get thumbnail match state .core .thumbnail_service - .get_thumbnail(&object_id, &blob_hash, thumb_size.into(), &blob_path) + .get_thumbnail_from_bytes(&object_id, &blob_hash, thumb_size.into(), original_bytes) .await { Ok(data) => { let etag = format!("\"thumb-{}-{:?}\"", object_id, thumb_size); Response::builder() .status(StatusCode::OK) - .header(header::CONTENT_TYPE, "image/webp") + .header(header::CONTENT_TYPE, "image/jpeg") .header(header::CONTENT_LENGTH, data.len()) .header(header::CACHE_CONTROL, "public, max-age=31536000, immutable") .header(header::ETAG, etag)