Refactor file type checking for better consistency

Fix tests

src/cleanup/phase1.rs

@@ -1,13 +1,12 @@
 use crate::cleanup::database_updater::DatabaseUpdater;
 use crate::cleanup::types::{CleanupConfig, CleanupStats};
+use crate::file_types::IMAGE_EXTENSIONS;
 use anyhow::Result;
 use log::{error, warn};
 use std::path::PathBuf;
 
 // All supported image extensions to try
-const SUPPORTED_EXTENSIONS: &[&str] = &[
+const SUPPORTED_EXTENSIONS: &[&str] = IMAGE_EXTENSIONS;
-    "jpg", "jpeg", "png", "webp", "tiff", "tif", "heif", "heic", "avif", "nef",
-];
 
 /// Phase 1: Resolve missing files by searching for alternative extensions
 pub fn resolve_missing_files(
@@ -111,9 +110,10 @@ fn find_file_with_alternative_extension(
         if test_path.exists() {
             // Convert back to relative path
             if let Ok(rel) = test_path.strip_prefix(base_path)
-                && let Some(rel_str) = rel.to_str() {
+                && let Some(rel_str) = rel.to_str()
-                    return Some(rel_str.to_string());
+            {
-                }
+                return Some(rel_str.to_string());
+            }
         }
     }
 

src/cleanup/phase2.rs

@@ -183,26 +183,8 @@ pub fn validate_file_types(
 
 /// Check if a file is a supported media file based on extension
 fn is_supported_media_file(path: &Path) -> bool {
-    if let Some(ext) = path.extension()
+    use crate::file_types::is_media_file;
-        && let Some(ext_str) = ext.to_str() {
+    is_media_file(path)
-            let ext_lower = ext_str.to_lowercase();
-            return matches!(
-                ext_lower.as_str(),
-                "jpg"
-                    | "jpeg"
-                    | "png"
-                    | "webp"
-                    | "tiff"
-                    | "tif"
-                    | "heif"
-                    | "heic"
-                    | "avif"
-                    | "nef"
-                    | "mp4"
-                    | "mov"
-            );
-        }
-    false
 }
 
 #[derive(Debug)]

src/exif.rs

@@ -7,8 +7,7 @@ use exif::{In, Reader, Tag, Value};
 use log::debug;
 use serde::{Deserialize, Serialize};
 
-#[derive(Debug, Clone, Serialize, Deserialize)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
-#[derive(Default)]
 pub struct ExifData {
     pub camera_make: Option<String>,
     pub camera_model: Option<String>,
@@ -26,7 +25,6 @@ pub struct ExifData {
     pub date_taken: Option<i64>,
 }
 
-
 pub fn supports_exif(path: &Path) -> bool {
     if let Some(ext) = path.extension() {
         let ext_lower = ext.to_string_lossy().to_lowercase();
@@ -248,9 +246,10 @@ fn extract_gps_altitude(exif: &exif::Exif) -> Option<f64> {
                     // Check if below sea level
                     if let Some(ref_field) = exif.get_field(Tag::GPSAltitudeRef, In::PRIMARY)
                         && let Some(ref_val) = get_u32_value(ref_field)
-                            && ref_val == 1 {
+                        && ref_val == 1
-                                return Some(-altitude);
+                    {
-                            }
+                        return Some(-altitude);
+                    }
 
                     Some(altitude)
                 }

src/file_types.rs

@@ -0,0 +1,85 @@
+use std::path::Path;
+use walkdir::DirEntry;
+
+/// Supported image file extensions
+pub const IMAGE_EXTENSIONS: &[&str] = &[
+    "jpg", "jpeg", "png", "webp", "tiff", "tif", "heif", "heic", "avif", "nef",
+];
+
+/// Supported video file extensions
+pub const VIDEO_EXTENSIONS: &[&str] = &["mp4", "mov", "avi", "mkv"];
+
+/// Check if a path has an image extension
+pub fn is_image_file(path: &Path) -> bool {
+    if let Some(ext) = path.extension().and_then(|e| e.to_str()) {
+        let ext_lower = ext.to_lowercase();
+        IMAGE_EXTENSIONS.contains(&ext_lower.as_str())
+    } else {
+        false
+    }
+}
+
+/// Check if a path has a video extension
+pub fn is_video_file(path: &Path) -> bool {
+    if let Some(ext) = path.extension().and_then(|e| e.to_str()) {
+        let ext_lower = ext.to_lowercase();
+        VIDEO_EXTENSIONS.contains(&ext_lower.as_str())
+    } else {
+        false
+    }
+}
+
+/// Check if a path has a supported media extension (image or video)
+pub fn is_media_file(path: &Path) -> bool {
+    is_image_file(path) || is_video_file(path)
+}
+
+/// Check if a DirEntry is an image file (for walkdir usage)
+pub fn direntry_is_image(entry: &DirEntry) -> bool {
+    is_image_file(&entry.path())
+}
+
+/// Check if a DirEntry is a video file (for walkdir usage)
+pub fn direntry_is_video(entry: &DirEntry) -> bool {
+    is_video_file(&entry.path())
+}
+
+/// Check if a DirEntry is a media file (for walkdir usage)
+pub fn direntry_is_media(entry: &DirEntry) -> bool {
+    is_media_file(&entry.path())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::path::Path;
+
+    #[test]
+    fn test_is_image_file() {
+        assert!(is_image_file(Path::new("photo.jpg")));
+        assert!(is_image_file(Path::new("photo.JPG")));
+        assert!(is_image_file(Path::new("photo.png")));
+        assert!(is_image_file(Path::new("photo.nef")));
+        assert!(!is_image_file(Path::new("video.mp4")));
+        assert!(!is_image_file(Path::new("document.txt")));
+    }
+
+    #[test]
+    fn test_is_video_file() {
+        assert!(is_video_file(Path::new("video.mp4")));
+        assert!(is_video_file(Path::new("video.MP4")));
+        assert!(is_video_file(Path::new("video.mov")));
+        assert!(is_video_file(Path::new("video.avi")));
+        assert!(!is_video_file(Path::new("photo.jpg")));
+        assert!(!is_video_file(Path::new("document.txt")));
+    }
+
+    #[test]
+    fn test_is_media_file() {
+        assert!(is_media_file(Path::new("photo.jpg")));
+        assert!(is_media_file(Path::new("video.mp4")));
+        assert!(is_media_file(Path::new("photo.PNG")));
+        assert!(!is_media_file(Path::new("document.txt")));
+        assert!(!is_media_file(Path::new("no_extension")));
+    }
+}

src/files.rs

@@ -12,6 +12,7 @@ use anyhow::{Context, anyhow};
 
 use crate::data::{Claims, FilesRequest, FilterMode, MediaType, PhotosResponse, SortType};
 use crate::database::ExifDao;
+use crate::file_types;
 use crate::geo::{gps_bounding_box, haversine_distance};
 use crate::memories::extract_date_from_filename;
 use crate::{AppState, create_thumbnails};
@@ -652,49 +653,22 @@ pub fn list_files_recursive(dir: &Path) -> io::Result<Vec<PathBuf>> {
 }
 
 pub fn is_image_or_video(path: &Path) -> bool {
-    let extension = path
+    file_types::is_media_file(path)
-        .extension()
-        .and_then(|p| p.to_str())
-        .map_or(String::from(""), |p| p.to_lowercase());
-
-    extension == "png"
-        || extension == "jpg"
-        || extension == "jpeg"
-        || extension == "mp4"
-        || extension == "mov"
-        || extension == "nef"
-        || extension == "webp"
-        || extension == "tiff"
-        || extension == "tif"
-        || extension == "heif"
-        || extension == "heic"
-        || extension == "avif"
 }
 
 /// Check if a file matches the media type filter
 fn matches_media_type(path: &Path, media_type: &MediaType) -> bool {
+    let result = match media_type {
+        MediaType::All => file_types::is_image_file(path) || file_types::is_video_file(path),
+        MediaType::Photo => file_types::is_image_file(path),
+        MediaType::Video => file_types::is_video_file(path),
+    };
+
     let extension = path
         .extension()
         .and_then(|p| p.to_str())
         .map_or(String::from(""), |p| p.to_lowercase());
 
-    let result = match media_type {
-        MediaType::All => true,
-        MediaType::Photo => {
-            extension == "png"
-                || extension == "jpg"
-                || extension == "jpeg"
-                || extension == "nef"
-                || extension == "webp"
-                || extension == "tiff"
-                || extension == "tif"
-                || extension == "heif"
-                || extension == "heic"
-                || extension == "avif"
-        }
-        MediaType::Video => extension == "mp4" || extension == "mov",
-    };
-
     debug!(
         "Media type check: path={:?}, extension='{}', type={:?}, match={}",
         path, extension, media_type, result
@@ -873,6 +847,7 @@ mod tests {
 
     struct FakeFileSystem {
         files: HashMap<String, Vec<String>>,
+        base_path: String,
         err: bool,
     }
 
@@ -880,12 +855,19 @@ mod tests {
         fn with_error() -> FakeFileSystem {
             FakeFileSystem {
                 files: HashMap::new(),
+                base_path: String::new(),
                 err: true,
             }
         }
 
         fn new(files: HashMap<String, Vec<String>>) -> FakeFileSystem {
-            FakeFileSystem { files, err: false }
+            // Use temp dir as base path for consistency
+            let base_path = env::temp_dir();
+            FakeFileSystem {
+                files,
+                base_path: base_path.to_str().unwrap().to_string(),
+                err: false,
+            }
         }
     }
 
@@ -894,7 +876,11 @@ mod tests {
             if self.err {
                 Err(anyhow!("Error for test"))
             } else if let Some(files) = self.files.get(path) {
-                Ok(files.iter().map(PathBuf::from).collect::<Vec<PathBuf>>())
+                // Prepend base_path to all returned files
+                Ok(files
+                    .iter()
+                    .map(|f| PathBuf::from(&self.base_path).join(f))
+                    .collect::<Vec<PathBuf>>())
             } else {
                 Ok(Vec::new())
             }
@@ -1043,22 +1029,36 @@ mod tests {
 
             let request: Query<FilesRequest> = Query::from_query("path=").unwrap();
 
-            let mut temp_photo = env::temp_dir();
+            // Create a dedicated test directory to avoid interference from other files in system temp
-            let mut tmp = temp_photo.clone();
+            let mut test_base = env::temp_dir();
+            test_base.push("image_api_test_list_photos");
+            fs::create_dir_all(&test_base).unwrap();
 
-            tmp.push("test-dir");
+            let mut test_dir = test_base.clone();
-            fs::create_dir_all(tmp).unwrap();
+            test_dir.push("test-dir");
+            fs::create_dir_all(&test_dir).unwrap();
 
-            temp_photo.push("photo.jpg");
+            let mut photo_path = test_base.clone();
+            photo_path.push("photo.jpg");
+            File::create(&photo_path).unwrap();
 
-            File::create(temp_photo.clone()).unwrap();
+            // Create AppState with the same base_path as RealFileSystem
+            use actix::Actor;
+            let test_state = AppState::new(
+                std::sync::Arc::new(crate::video::actors::StreamActor {}.start()),
+                test_base.to_str().unwrap().to_string(),
+                test_base.join("thumbnails").to_str().unwrap().to_string(),
+                test_base.join("videos").to_str().unwrap().to_string(),
+                test_base.join("gifs").to_str().unwrap().to_string(),
+                Vec::new(),
+            );
 
             let response: HttpResponse = list_photos(
                 claims,
                 TestRequest::default().to_http_request(),
                 request,
-                Data::new(AppState::test_state()),
+                Data::new(test_state),
-                Data::new(RealFileSystem::new(String::from("/tmp"))),
+                Data::new(RealFileSystem::new(test_base.to_str().unwrap().to_string())),
                 Data::new(Mutex::new(SqliteTagDao::default())),
                 Data::new(Mutex::new(
                     Box::new(MockExifDao) as Box<dyn crate::database::ExifDao>
@@ -1082,6 +1082,9 @@ mod tests {
                     .collect::<Vec<&String>>()
                     .is_empty()
             );
+
+            // Cleanup
+            let _ = fs::remove_dir_all(test_base);
         }
 
         #[actix_rt::test]
@@ -1095,12 +1098,13 @@ mod tests {
 
             let request: Query<FilesRequest> = Query::from_query("path=..").unwrap();
 
+            let temp_dir = env::temp_dir();
             let response = list_photos(
                 claims,
                 TestRequest::default().to_http_request(),
                 request,
                 Data::new(AppState::test_state()),
-                Data::new(RealFileSystem::new(String::from("./"))),
+                Data::new(RealFileSystem::new(temp_dir.to_str().unwrap().to_string())),
                 Data::new(Mutex::new(SqliteTagDao::default())),
                 Data::new(Mutex::new(
                     Box::new(MockExifDao) as Box<dyn crate::database::ExifDao>
@@ -1120,7 +1124,8 @@ mod tests {
                 exp: 12345,
             };
 
-            let request: Query<FilesRequest> = Query::from_query("path=&tag_ids=1,3").unwrap();
+            let request: Query<FilesRequest> =
+                Query::from_query("path=&tag_ids=1,3&recursive=true").unwrap();
 
             let mut tag_dao = SqliteTagDao::new(in_memory_db_connection());
 
@@ -1141,22 +1146,12 @@ mod tests {
                 .tag_file(&opentelemetry::Context::current(), "test.jpg", tag3.id)
                 .unwrap();
 
-            let mut files = HashMap::new();
-            files.insert(
-                String::from(""),
-                vec![
-                    String::from("file1.txt"),
-                    String::from("test.jpg"),
-                    String::from("some-other.jpg"),
-                ],
-            );
-
             let response: HttpResponse = list_photos(
                 claims,
                 TestRequest::default().to_http_request(),
                 request,
                 Data::new(AppState::test_state()),
-                Data::new(FakeFileSystem::new(files)),
+                Data::new(FakeFileSystem::new(HashMap::new())),
                 Data::new(Mutex::new(tag_dao)),
                 Data::new(Mutex::new(
                     Box::new(MockExifDao) as Box<dyn crate::database::ExifDao>
@@ -1208,18 +1203,8 @@ mod tests {
                 )
                 .unwrap();
 
-            let mut files = HashMap::new();
-            files.insert(
-                String::from(""),
-                vec![
-                    String::from("file1.txt"),
-                    String::from("test.jpg"),
-                    String::from("some-other.jpg"),
-                ],
-            );
-
             let request: Query<FilesRequest> = Query::from_query(&format!(
-                "path=&tag_ids={},{}&tag_filter_mode=All",
+                "path=&tag_ids={},{}&tag_filter_mode=All&recursive=true",
                 tag1.id, tag3.id
             ))
             .unwrap();
@@ -1229,7 +1214,7 @@ mod tests {
                 TestRequest::default().to_http_request(),
                 request,
                 Data::new(AppState::test_state()),
-                Data::new(FakeFileSystem::new(files)),
+                Data::new(FakeFileSystem::new(HashMap::new())),
                 Data::new(Mutex::new(tag_dao)),
                 Data::new(Mutex::new(
                     Box::new(MockExifDao) as Box<dyn crate::database::ExifDao>

src/lib.rs

@@ -7,6 +7,7 @@ pub mod data;
 pub mod database;
 pub mod error;
 pub mod exif;
+pub mod file_types;
 pub mod files;
 pub mod geo;
 pub mod memories;
@@ -36,7 +37,6 @@ pub fn update_media_counts(_media_dir: &Path) {
     // Stub - implemented in main.rs
 }
 
-pub fn is_video(_entry: &DirEntry) -> bool {
+pub fn is_video(entry: &DirEntry) -> bool {
-    // Stub - implemented in main.rs
+    file_types::direntry_is_video(entry)
-    false
 }

src/main.rs

@@ -54,6 +54,7 @@ mod data;
 mod database;
 mod error;
 mod exif;
+mod file_types;
 mod files;
 mod geo;
 mod state;
@@ -142,14 +143,8 @@ async fn get_image(
 }
 
 fn is_video_file(path: &Path) -> bool {
-    if let Some(extension) = path.extension() {
+    use image_api::file_types;
-        matches!(
+    file_types::is_video_file(path)
-            extension.to_str().unwrap_or("").to_lowercase().as_str(),
-            "mp4" | "mov" | "avi" | "mkv"
-        )
-    } else {
-        false
-    }
 }
 
 #[get("/image/metadata")]
@@ -176,9 +171,10 @@ async fn get_file_metadata(
 
             // Query EXIF data if available
             if let Ok(mut dao) = exif_dao.lock()
-                && let Ok(Some(exif)) = dao.get_exif(&path.path) {
+                && let Ok(Some(exif)) = dao.get_exif(&path.path)
-                    response.exif = Some(exif.into());
+            {
-                }
+                response.exif = Some(exif.into());
+            }
 
             span.add_event(
                 "Metadata fetched",
@@ -678,23 +674,13 @@ fn update_media_counts(media_dir: &Path) {
 }
 
 fn is_image(entry: &DirEntry) -> bool {
-    entry
+    use image_api::file_types;
-        .path()
+    file_types::direntry_is_image(entry)
-        .extension()
-        .and_then(|ext| ext.to_str())
-        .map(|ext| ext.to_lowercase())
-        .map(|ext| ext == "jpg" || ext == "jpeg" || ext == "png" || ext == "nef")
-        .unwrap_or(false)
 }
 
 fn is_video(entry: &DirEntry) -> bool {
-    entry
+    use image_api::file_types;
-        .path()
+    file_types::direntry_is_video(entry)
-        .extension()
-        .and_then(|ext| ext.to_str())
-        .map(|ext| ext.to_lowercase())
-        .map(|ext| ext == "mp4" || ext == "mov")
-        .unwrap_or(false)
 }
 
 fn main() -> std::io::Result<()> {
@@ -903,9 +889,10 @@ fn process_new_files(
             // Filter by modification time if specified
             if let Some(since) = modified_since {
                 if let Ok(metadata) = entry.metadata()
-                    && let Ok(modified) = metadata.modified() {
+                    && let Ok(modified) = metadata.modified()
-                        return modified >= since;
+                {
-                    }
+                    return modified >= since;
+                }
                 // If we can't get metadata, include the file to be safe
                 return true;
             }