Merge pull request 'Feature/no original fallback' (#158) from feature/no-original-fallback into master

Reviewed-on: #158

.idea/image-api.iml

@@ -3,6 +3,7 @@
   <component name="NewModuleRootManager">
     <content url="file://$MODULE_DIR$">
       <sourceFolder url="file://$MODULE_DIR$/src" isTestSource="false" />
+      <sourceFolder url="file://$MODULE_DIR$/tests" isTestSource="true" />
       <excludeFolder url="file://$MODULE_DIR$/.idea/dataSources" />
       <excludeFolder url="file://$MODULE_DIR$/target" />
     </content>

src/handlers/image.rs

@@ -94,100 +94,24 @@ pub async fn get_image(
             image_size
         };
 
-        if image_size == PhotoSize::Large {
+        if image_size == PhotoSize::Large || image_size == PhotoSize::XLarge {
-            let relative_path = path
+            let tier = if image_size == PhotoSize::Large {
-                .strip_prefix(&library.root_path)
+                PreviewTier::Large
-                .expect("Error stripping library root prefix from large preview");
+            } else {
-            let relative_path_str = relative_path.to_string_lossy().replace('\\', "/");
+                PreviewTier::XLarge
-            let thumbs = Path::new(&app_state.thumbnail_path);
-            let large_dir = thumbs.join("_large");
-
-            // Lookup chain mirrors the Thumb branch — hash-keyed first so
-            // multi-library deployments share derivative bytes across
-            // libraries, then library-scoped legacy as the fallback for
-            // rows that aren't hashed yet.
-            let hash_large_path: Option<PathBuf> = {
-                let mut dao = exif_dao.lock().expect("Unable to lock ExifDao");
-                match dao.get_exif(&relative_path_str) {
-                    Ok(Some(row)) => row
-                        .content_hash
-                        .as_deref()
-                        .map(|h| content_hash::large_preview_path(thumbs, h)),
-                    _ => None,
-                }
             };
-            let scoped_legacy_large_path =
+            if let Some(resp) = serve_preview(
-                content_hash::library_scoped_legacy_path(&large_dir, library.id, relative_path);
+                tier,
-
+                library,
-            let existing = hash_large_path
+                &path,
-                .as_ref()
+                Path::new(&app_state.thumbnail_path),
-                .filter(|p| p.exists())
+                &exif_dao,
-                .cloned()
+                &request,
-                .or_else(|| {
+            )
-                    if scoped_legacy_large_path.exists() {
+            .await
-                        Some(scoped_legacy_large_path.clone())
-                    } else {
-                        None
-                    }
-                });
-
-            if let Some(found) = existing
-                && let Ok(file) = NamedFile::open(&found)
             {
                 span.set_status(Status::Ok);
-                return file
+                return resp;
-                    .use_etag(true)
-                    .use_last_modified(true)
-                    .prefer_utf8(true)
-                    .into_response(&request);
-            }
-
-            // Cache miss — generate. Resize + JPEG-encode can take 100–500ms
-            // for a 24MP source (longer for RAW), so run on the blocking pool
-            // to keep the actix worker free. Prefer the hash-keyed
-            // destination when a hash is known so the result is reusable
-            // across libraries that hold the same bytes.
-            let dest = hash_large_path
-                .clone()
-                .unwrap_or_else(|| scoped_legacy_large_path.clone());
-            let src = path.clone();
-            let dest_for_block = dest.clone();
-            let generated = web::block(move || {
-                if let Some(parent) = dest_for_block.parent() {
-                    std::fs::create_dir_all(parent)?;
-                }
-                // Write to a sibling tempfile then atomically rename so a
-                // concurrent reader never observes a half-written JPEG.
-                let tmp = dest_for_block.with_extension("jpg.tmp");
-                crate::thumbnails::generate_large_preview(&src, &tmp)?;
-                std::fs::rename(&tmp, &dest_for_block)?;
-                Ok::<(), std::io::Error>(())
-            })
-            .await;
-
-            match generated {
-                Ok(Ok(())) => {
-                    if let Ok(file) = NamedFile::open(&dest) {
-                        span.set_status(Status::Ok);
-                        return file
-                            .use_etag(true)
-                            .use_last_modified(true)
-                            .prefer_utf8(true)
-                            .into_response(&request);
-                    }
-                }
-                Ok(Err(e)) => {
-                    warn!(
-                        "Large preview generation failed for {:?}: {} — falling back to original",
-                        path, e
-                    );
-                }
-                Err(e) => {
-                    warn!(
-                        "Large preview blocking-pool error for {:?}: {} — falling back to original",
-                        path, e
-                    );
-                }
             }
             // Fall through to the Full branch below so the caller gets
             // *something* useful (the original bytes — or the RAW
@@ -195,94 +119,6 @@ pub async fn get_image(
             // unrenderable RAW containers) instead of a 404.
             image_size = PhotoSize::Full;
         }
-
-        if image_size == PhotoSize::XLarge {
-            let relative_path = path
-                .strip_prefix(&library.root_path)
-                .expect("Error stripping library root prefix from xlarge preview");
-            let relative_path_str = relative_path.to_string_lossy().replace('\\', "/");
-            let thumbs = Path::new(&app_state.thumbnail_path);
-            let xlarge_dir = thumbs.join("_xlarge");
-
-            let hash_xlarge_path: Option<PathBuf> = {
-                let mut dao = exif_dao.lock().expect("Unable to lock ExifDao");
-                match dao.get_exif(&relative_path_str) {
-                    Ok(Some(row)) => row
-                        .content_hash
-                        .as_deref()
-                        .map(|h| content_hash::xlarge_preview_path(thumbs, h)),
-                    _ => None,
-                }
-            };
-            let scoped_legacy_xlarge_path =
-                content_hash::library_scoped_legacy_path(&xlarge_dir, library.id, relative_path);
-
-            let existing = hash_xlarge_path
-                .as_ref()
-                .filter(|p| p.exists())
-                .cloned()
-                .or_else(|| {
-                    if scoped_legacy_xlarge_path.exists() {
-                        Some(scoped_legacy_xlarge_path.clone())
-                    } else {
-                        None
-                    }
-                });
-
-            if let Some(found) = existing
-                && let Ok(file) = NamedFile::open(&found)
-            {
-                span.set_status(Status::Ok);
-                return file
-                    .use_etag(true)
-                    .use_last_modified(true)
-                    .prefer_utf8(true)
-                    .into_response(&request);
-            }
-
-            let dest = hash_xlarge_path
-                .clone()
-                .unwrap_or_else(|| scoped_legacy_xlarge_path.clone());
-            let src = path.clone();
-            let dest_for_block = dest.clone();
-            let generated = web::block(move || {
-                if let Some(parent) = dest_for_block.parent() {
-                    std::fs::create_dir_all(parent)?;
-                }
-                let tmp = dest_for_block.with_extension("jpg.tmp");
-                crate::thumbnails::generate_xlarge_preview(&src, &tmp)?;
-                std::fs::rename(&tmp, &dest_for_block)?;
-                Ok::<(), std::io::Error>(())
-            })
-            .await;
-
-            match generated {
-                Ok(Ok(())) => {
-                    if let Ok(file) = NamedFile::open(&dest) {
-                        span.set_status(Status::Ok);
-                        return file
-                            .use_etag(true)
-                            .use_last_modified(true)
-                            .prefer_utf8(true)
-                            .into_response(&request);
-                    }
-                }
-                Ok(Err(e)) => {
-                    warn!(
-                        "XLarge preview generation failed for {:?}: {} — falling back to original",
-                        path, e
-                    );
-                }
-                Err(e) => {
-                    warn!(
-                        "XLarge preview blocking-pool error for {:?}: {} — falling back to original",
-                        path, e
-                    );
-                }
-            }
-            image_size = PhotoSize::Full;
-        }
-
         if image_size == PhotoSize::Thumb {
             let relative_path = path
                 .strip_prefix(&library.root_path)
@@ -375,6 +211,30 @@ pub async fn get_image(
                     .prefer_utf8(true)
                     .into_response(&request);
             }
+
+            // No thumb on disk (never generated, or an `.unsupported`
+            // sentinel from a failed scan). Never stream the original into
+            // a thumb slot: a 12432×3504 panorama decodes to 174MB, past
+            // Android's 100MB draw limit, and crashes the app natively.
+            // The large tier is capped at 2048px; if even that can't be
+            // made, the client gets a 404 and shows a blank tile.
+            if !file_types::is_video_file(&path) {
+                if let Some(resp) = serve_preview(
+                    PreviewTier::Large,
+                    library,
+                    &path,
+                    Path::new(&app_state.thumbnail_path),
+                    &exif_dao,
+                    &request,
+                )
+                .await
+                {
+                    span.set_status(Status::Ok);
+                    return resp;
+                }
+                span.set_status(Status::error("Thumbnail unavailable"));
+                return HttpResponse::NotFound().finish();
+            }
         }
 
         // Full-size requests for RAW formats (NEF/CR2/ARW/etc.) can't just
@@ -414,6 +274,143 @@ pub async fn get_image(
     }
 }
 
+/// On-demand resized tiers served by [`serve_preview`].
+#[derive(Clone, Copy)]
+enum PreviewTier {
+    Large,
+    XLarge,
+}
+
+impl PreviewTier {
+    fn name(self) -> &'static str {
+        match self {
+            PreviewTier::Large => "Large",
+            PreviewTier::XLarge => "XLarge",
+        }
+    }
+
+    fn legacy_dir(self) -> &'static str {
+        match self {
+            PreviewTier::Large => "_large",
+            PreviewTier::XLarge => "_xlarge",
+        }
+    }
+
+    fn hash_path(self, thumbs: &Path, hash: &str) -> PathBuf {
+        match self {
+            PreviewTier::Large => content_hash::large_preview_path(thumbs, hash),
+            PreviewTier::XLarge => content_hash::xlarge_preview_path(thumbs, hash),
+        }
+    }
+
+    fn generate(self, src: &Path, dest: &Path) -> std::io::Result<()> {
+        match self {
+            PreviewTier::Large => crate::thumbnails::generate_large_preview(src, dest),
+            PreviewTier::XLarge => crate::thumbnails::generate_xlarge_preview(src, dest),
+        }
+    }
+}
+
+/// Serve the cached `tier` preview of `path`, generating it on a miss.
+/// `None` when it can't be produced; the caller picks the fallback.
+async fn serve_preview(
+    tier: PreviewTier,
+    library: &libraries::Library,
+    path: &Path,
+    thumbs: &Path,
+    exif_dao: &Mutex<Box<dyn ExifDao>>,
+    request: &HttpRequest,
+) -> Option<HttpResponse> {
+    let relative_path = path
+        .strip_prefix(&library.root_path)
+        .expect("Error stripping library root prefix from preview");
+    let relative_path_str = relative_path.to_string_lossy().replace('\\', "/");
+
+    // Lookup chain mirrors the Thumb branch — hash-keyed first so
+    // multi-library deployments share derivative bytes across
+    // libraries, then library-scoped legacy as the fallback for
+    // rows that aren't hashed yet.
+    let hash_path: Option<PathBuf> = {
+        let mut dao = exif_dao.lock().expect("Unable to lock ExifDao");
+        match dao.get_exif(&relative_path_str) {
+            Ok(Some(row)) => row
+                .content_hash
+                .as_deref()
+                .map(|h| tier.hash_path(thumbs, h)),
+            _ => None,
+        }
+    };
+    let scoped_legacy_path = content_hash::library_scoped_legacy_path(
+        &thumbs.join(tier.legacy_dir()),
+        library.id,
+        relative_path,
+    );
+
+    let serve = |p: &Path| {
+        NamedFile::open(p).ok().map(|file| {
+            file.use_etag(true)
+                .use_last_modified(true)
+                .prefer_utf8(true)
+                .into_response(request)
+        })
+    };
+
+    let existing = hash_path
+        .as_ref()
+        .filter(|p| p.exists())
+        .or_else(|| Some(&scoped_legacy_path).filter(|p| p.exists()));
+    if let Some(resp) = existing.and_then(|p| serve(p)) {
+        return Some(resp);
+    }
+
+    // Cache miss — generate. Resize + JPEG-encode can take 100–500ms
+    // for a 24MP source (longer for RAW), so run on the blocking pool
+    // to keep the actix worker free. Prefer the hash-keyed
+    // destination when a hash is known so the result is reusable
+    // across libraries that hold the same bytes.
+    let dest = hash_path.unwrap_or(scoped_legacy_path);
+    let src = path.to_path_buf();
+    let dest_for_block = dest.clone();
+    let generated = web::block(move || generate_preview(tier, &src, &dest_for_block)).await;
+
+    match generated {
+        Ok(Ok(())) => {}
+        Ok(Err(e)) => warn!(
+            "{} preview generation failed for {:?}: {}",
+            tier.name(),
+            path,
+            e
+        ),
+        Err(e) => warn!(
+            "{} preview blocking-pool error for {:?}: {}",
+            tier.name(),
+            path,
+            e
+        ),
+    }
+    serve(&dest)
+}
+
+/// Generate `tier` for `src` into a per-call tempfile, then atomically
+/// rename it to `dest`, so a concurrent reader never sees a half-written
+/// JPEG. The tempfile name must be unique: clients fire the same request
+/// twice at once (tile remounts), and a shared `<dest>.tmp` let the first
+/// rename steal the second's file — "cannot find the file specified
+/// (os error 2)".
+fn generate_preview(tier: PreviewTier, src: &Path, dest: &Path) -> std::io::Result<()> {
+    let parent = dest
+        .parent()
+        .ok_or_else(|| std::io::Error::other("preview path has no parent"))?;
+    std::fs::create_dir_all(parent)?;
+    let tmp = tempfile::Builder::new()
+        .suffix(".jpg.tmp")
+        .tempfile_in(parent)?
+        .into_temp_path();
+    tier.generate(src, &tmp)?;
+    tmp.persist(dest)?;
+    Ok(())
+}
+
 async fn create_circular_thumbnail(
     thumb_path: &Path,
     thumbs_dir: &str,
@@ -1253,3 +1250,99 @@ pub async fn upload_image(
 
     HttpResponse::Ok().finish()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::database::SqliteExifDao;
+    use crate::database::test::in_memory_db_connection;
+    use actix_web::test::TestRequest;
+    use image::GenericImageView;
+
+    fn library(root: &Path) -> libraries::Library {
+        libraries::Library {
+            id: libraries::PRIMARY_LIBRARY_ID,
+            name: "main".to_string(),
+            root_path: root.to_string_lossy().to_string(),
+            enabled: true,
+            excluded_dirs: Vec::new(),
+        }
+    }
+
+    fn exif_dao() -> Mutex<Box<dyn ExifDao>> {
+        Mutex::new(Box::new(SqliteExifDao::from_connection(
+            in_memory_db_connection(),
+        )))
+    }
+
+    /// Write a solid-color JPEG of the given size under `root`.
+    fn write_jpeg(root: &Path, name: &str, w: u32, h: u32) -> PathBuf {
+        let path = root.join(name);
+        image::RgbImage::from_pixel(w, h, image::Rgb([90, 140, 200]))
+            .save(&path)
+            .unwrap();
+        path
+    }
+
+    #[test]
+    fn concurrent_generation_of_one_preview_all_succeed() {
+        let root = tempfile::tempdir().unwrap();
+        let thumbs = tempfile::tempdir().unwrap();
+        let src = write_jpeg(root.path(), "pano.jpg", 3000, 800);
+        let dest = thumbs.path().join("_large").join("ab").join("abcd.jpg");
+
+        // Clients fire the same preview twice at once; with a shared
+        // tempfile name the loser's rename failed with os error 2.
+        let barrier = std::sync::Barrier::new(8);
+        std::thread::scope(|s| {
+            let handles: Vec<_> = (0..8)
+                .map(|_| {
+                    s.spawn(|| {
+                        barrier.wait();
+                        generate_preview(PreviewTier::Large, &src, &dest)
+                    })
+                })
+                .collect();
+            for h in handles {
+                h.join().unwrap().expect("concurrent generation failed");
+            }
+        });
+
+        assert!(dest.exists());
+        let leftovers: Vec<_> = std::fs::read_dir(dest.parent().unwrap())
+            .unwrap()
+            .filter_map(Result::ok)
+            .filter(|e| e.file_name() != "abcd.jpg")
+            .collect();
+        assert!(leftovers.is_empty(), "stray tempfiles: {:?}", leftovers);
+    }
+
+    #[actix_rt::test]
+    async fn large_preview_caps_a_panorama_at_2048() {
+        let root = tempfile::tempdir().unwrap();
+        let thumbs = tempfile::tempdir().unwrap();
+        let lib = library(root.path());
+        let src = write_jpeg(root.path(), "pano.jpg", 6000, 1600);
+        let req = TestRequest::default().to_http_request();
+
+        serve_preview(
+            PreviewTier::Large,
+            &lib,
+            &src,
+            thumbs.path(),
+            &exif_dao(),
+            &req,
+        )
+        .await
+        .expect("preview served");
+
+        // Unhashed row → library-scoped legacy path under `_large/`.
+        let dest = content_hash::library_scoped_legacy_path(
+            &thumbs.path().join("_large"),
+            lib.id,
+            Path::new("pano.jpg"),
+        );
+        let (w, h) = image::open(dest).unwrap().dimensions();
+        assert_eq!((w, h), (2048, 546));
+    }
+}