Move list_photos to files module

Added some tests, refactored the error handling/logging, and refactored
the extension tests.

src/files.rs

@@ -1,21 +1,68 @@
 use std::fs::read_dir;
 use std::io;
-use std::io::Error;
 use std::path::{Path, PathBuf};
 
+use ::anyhow;
+use anyhow::{anyhow, Context};
+
+use actix_web::web::{HttpResponse, Query};
+
+use log::{debug, error};
+
+use crate::data::{Claims, PhotosResponse, ThumbnailRequest};
+
 use path_absolutize::*;
 
+pub async fn list_photos(_: Claims, req: Query<ThumbnailRequest>) -> HttpResponse {
+    let path = &req.path;
+    if let Some(path) = is_valid_path(path) {
+        debug!("Valid path: {:?}", path);
+        let files = list_files(&path).unwrap_or_default();
+
+        let photos = files
+            .iter()
+            .filter(|&f| {
+                f.metadata().map_or_else(
+                    |e| {
+                        error!("Failed getting file metadata: {:?}", e);
+                        false
+                    },
+                    |md| md.is_file(),
+                )
+            })
+            .map(|path: &PathBuf| {
+                let relative = path
+                    .strip_prefix(dotenv::var("BASE_PATH").unwrap())
+                    .unwrap();
+                relative.to_path_buf()
+            })
+            .map(|f| f.to_str().unwrap().to_string())
+            .collect::<Vec<String>>();
+
+        let dirs = files
+            .iter()
+            .filter(|&f| f.metadata().map_or(false, |md| md.is_dir()))
+            .map(|path: &PathBuf| {
+                let relative = path
+                    .strip_prefix(dotenv::var("BASE_PATH").unwrap())
+                    .unwrap();
+                relative.to_path_buf()
+            })
+            .map(|f| f.to_str().unwrap().to_string())
+            .collect::<Vec<String>>();
+
+        HttpResponse::Ok().json(PhotosResponse { photos, dirs })
+    } else {
+        error!("Bad photos request: {}", req.path);
+        HttpResponse::BadRequest().finish()
+    }
+}
+
 pub fn list_files(dir: &Path) -> io::Result<Vec<PathBuf>> {
     let files = read_dir(dir)?
-        .map(|res| res.unwrap())
+        .filter_map(|res| res.ok())
         .filter(|entry| is_image_or_video(&entry.path()) || entry.file_type().unwrap().is_dir())
         .map(|entry| entry.path())
-        .map(|path: PathBuf| {
-            let relative = path
-                .strip_prefix(dotenv::var("BASE_PATH").unwrap())
-                .unwrap();
-            relative.to_path_buf()
-        })
         .collect::<Vec<PathBuf>>();
 
     Ok(files)
@@ -42,29 +89,42 @@ pub fn is_valid_path(path: &str) -> Option<PathBuf> {
 }
 
 fn is_valid_full_path(base: &Path, path: &str) -> Option<PathBuf> {
-    let mut path = PathBuf::from(path);
-    if path.is_relative() {
+    debug!("Base: {:?}. Path: {}", base, path);
+
+    let path = PathBuf::from(path);
+    let mut path = if path.is_relative() {
         let mut full_path = PathBuf::from(base);
         full_path.push(&path);
-        is_path_above_base_dir(base, &mut full_path).ok()
-    } else if let Ok(path) = is_path_above_base_dir(base, &mut path) {
-        Some(path)
+        full_path
     } else {
-        None
+        path
+    };
+
+    match is_path_above_base_dir(base, &mut path) {
+        Ok(path) => Some(path),
+        Err(e) => {
+            error!("{}", e);
+            None
+        }
     }
 }
 
-fn is_path_above_base_dir(base: &Path, full_path: &mut PathBuf) -> Result<PathBuf, Error> {
-    full_path.absolutize().and_then(|p| {
-        if p.starts_with(base) {
-            Ok(p.into_owned())
-        } else {
-            Err(io::Error::new(
-                io::ErrorKind::Other,
-                "Path below base directory",
-            ))
-        }
-    })
+fn is_path_above_base_dir(base: &Path, full_path: &mut PathBuf) -> anyhow::Result<PathBuf> {
+    full_path
+        .absolutize()
+        .with_context(|| format!("Unable to resolve absolute path: {:?}", full_path))
+        .map_or_else(
+            |e| Err(anyhow!(e)),
+            |p| {
+                if p.starts_with(base) && p.exists() {
+                    Ok(p.into_owned())
+                } else if !p.exists() {
+                    Err(anyhow!("Path does not exist: {:?}", p))
+                } else {
+                    Err(anyhow!("Path above base directory"))
+                }
+            },
+        )
 }
 
 #[cfg(test)]
@@ -74,6 +134,77 @@ mod tests {
 
     use super::*;
 
+    mod api {
+        use actix_web::{web::Query, HttpResponse};
+
+        use super::list_photos;
+        use crate::{
+            data::{Claims, PhotosResponse, ThumbnailRequest},
+            testhelpers::TypedBodyReader,
+        };
+
+        use std::fs;
+
+        fn setup() {
+            let _ = env_logger::builder().is_test(true).try_init();
+        }
+
+        #[actix_rt::test]
+        async fn test_list_photos() {
+            setup();
+
+            let claims = Claims {
+                sub: String::from("1"),
+                exp: 12345,
+            };
+
+            let request: Query<ThumbnailRequest> = Query::from_query("path=").unwrap();
+
+            std::env::set_var("BASE_PATH", "/tmp");
+            let mut temp_photo = std::env::temp_dir();
+            let mut tmp = temp_photo.clone();
+
+            tmp.push("test-dir");
+            fs::create_dir_all(tmp).unwrap();
+
+            temp_photo.push("photo.jpg");
+
+            fs::File::create(temp_photo).unwrap();
+
+            let response: HttpResponse = list_photos(claims, request).await;
+
+            let body: PhotosResponse = response.body().read_body();
+
+            assert_eq!(response.status(), 200);
+            assert!(body.photos.contains(&String::from("photo.jpg")));
+            assert!(body.dirs.contains(&String::from("test-dir")));
+            assert!(body
+                .photos
+                .iter()
+                .filter(|filename| !filename.ends_with(".png")
+                    && !filename.ends_with(".jpg")
+                    && !filename.ends_with(".jpeg"))
+                .collect::<Vec<&String>>()
+                .is_empty());
+        }
+
+        #[actix_rt::test]
+        async fn test_list_below_base_fails_400() {
+            setup();
+
+            let claims = Claims {
+                sub: String::from("1"),
+                exp: 12345,
+            };
+
+            let request: Query<ThumbnailRequest> = Query::from_query("path=..").unwrap();
+
+            let response = list_photos(claims, request).await;
+
+            assert_eq!(response.status(), 400);
+        }
+    }
+
     #[test]
     fn directory_traversal_test() {
         assert_eq!(None, is_valid_path("../"));
@@ -85,22 +216,24 @@ mod tests {
     }
 
     #[test]
-    fn build_from_relative_path_test() {
+    fn build_from_path_relative_to_base_test() {
         let base = env::temp_dir();
         let mut test_file = PathBuf::from(&base);
         test_file.push("test.png");
         File::create(test_file).unwrap();
 
         assert!(is_valid_full_path(&base, "test.png").is_some());
+    }
+
+    #[test]
+    fn build_from_relative_returns_none_if_directory_does_not_exist_test() {
+        let base = env::temp_dir();
 
         let path = "relative/path/test.png";
         let mut test_file = PathBuf::from(&base);
         test_file.push(path);
 
-        assert_eq!(
-            Some(PathBuf::from("/tmp/relative/path/test.png")),
-            is_valid_full_path(&base, path)
-        );
+        assert_eq!(None, is_valid_full_path(&base, path));
     }
 
     #[test]
@@ -112,51 +245,41 @@ mod tests {
 
         assert!(is_valid_full_path(&base, test_file.to_str().unwrap()).is_some());
 
-        let path = "relative/path/test.png";
-        let mut test_file = PathBuf::from(&base);
-        test_file.push(path);
-
         assert_eq!(
-            Some(PathBuf::from("/tmp/relative/path/test.png")),
-            is_valid_full_path(&base, path)
+            Some(PathBuf::from("/tmp/test.png")),
+            is_valid_full_path(&base, "/tmp/test.png")
         );
     }
 
-    #[test]
-    fn png_valid_extension_test() {
-        assert!(is_image_or_video(Path::new("image.png")));
-        assert!(is_image_or_video(Path::new("image.PNG")));
-        assert!(is_image_or_video(Path::new("image.pNg")));
+    macro_rules! extension_test {
+        ($name:ident, $filename:literal) => {
+            #[test]
+            fn $name() {
+                assert!(is_image_or_video(Path::new($filename)));
+            }
+        };
     }
 
-    #[test]
-    fn jpg_valid_extension_test() {
-        assert!(is_image_or_video(Path::new("image.jpeg")));
-        assert!(is_image_or_video(Path::new("image.JPEG")));
-        assert!(is_image_or_video(Path::new("image.jpg")));
-        assert!(is_image_or_video(Path::new("image.JPG")));
-    }
+    extension_test!(valid_png, "image.png");
+    extension_test!(valid_png_mixed_case, "image.pNg");
+    extension_test!(valid_png_upper_case, "image.PNG");
 
-    #[test]
-    fn mp4_valid_extension_test() {
-        assert!(is_image_or_video(Path::new("image.mp4")));
-        assert!(is_image_or_video(Path::new("image.mP4")));
-        assert!(is_image_or_video(Path::new("image.MP4")));
-    }
+    extension_test!(valid_jpeg, "image.jpeg");
+    extension_test!(valid_jpeg_upper_case, "image.JPEG");
+    extension_test!(valid_jpg, "image.jpg");
+    extension_test!(valid_jpg_upper_case, "image.JPG");
 
-    #[test]
-    fn mov_valid_extension_test() {
-        assert!(is_image_or_video(Path::new("image.mov")));
-        assert!(is_image_or_video(Path::new("image.MOV")));
-        assert!(is_image_or_video(Path::new("image.MoV")));
-    }
+    extension_test!(valid_mp4, "image.mp4");
+    extension_test!(valid_mp4_mixed_case, "image.mP4");
+    extension_test!(valid_mp4_upper_case, "image.MP4");
 
-    #[test]
-    fn nef_valid_extension_test() {
-        assert!(is_image_or_video(Path::new("image.nef")));
-        assert!(is_image_or_video(Path::new("image.NEF")));
-        assert!(is_image_or_video(Path::new("image.NeF")));
-    }
+    extension_test!(valid_mov, "image.mov");
+    extension_test!(valid_mov_mixed_case, "image.mOV");
+    extension_test!(valid_mov_upper_case, "image.MOV");
+
+    extension_test!(valid_nef, "image.nef");
+    extension_test!(valid_nef_mixed_case, "image.nEF");
+    extension_test!(valid_nef_upper_case, "image.NEF");
 
     #[test]
     fn hidden_file_not_valid_test() {