Merge pull request 'feature/sort-by-tag-count' (#32) from feature/sort-by-tag-count into master

Reviewed-on: #32

src/data/mod.rs

@@ -106,6 +106,8 @@ pub enum SortType {
     Shuffle,
     NameAsc,
     NameDesc,
+    TagCountAsc,
+    TagCountDesc,
 }
 
 #[derive(Deserialize)]

src/files.rs

@@ -19,9 +19,9 @@ use log::{debug, error, info, trace};
 use crate::data::{Claims, FilesRequest, FilterMode, PhotosResponse, SortType};
 use crate::{create_thumbnails, AppState};
 
-use crate::data::SortType::{NameAsc};
+use crate::data::SortType::NameAsc;
 use crate::error::IntoHttpError;
-use crate::tags::TagDao;
+use crate::tags::{FileWithTagCount, TagDao};
 use crate::video::StreamActor;
 use path_absolutize::*;
 use rand::prelude::SliceRandom;
@@ -68,6 +68,13 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
                 "Failed to get files with tag_ids: {:?} with filter_mode: {:?}",
                 tag_ids, filter_mode
             ))
+            .inspect(|files| {
+                debug!(
+                    "Found {:?} tagged files, filtering down by search path {:?}",
+                    files.len(),
+                    search_path
+                )
+            })
             .map(|tagged_files| {
                 tagged_files
                     .into_iter()
@@ -77,12 +84,12 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
                             return true;
                         }
 
-                        f.starts_with(&format!(
+                        f.file_name.starts_with(&format!(
                             "{}/",
                             search_path.strip_suffix('/').unwrap_or_else(|| search_path)
                         ))
                     })
-                    .collect::<Vec<String>>()
+                    .collect::<Vec<FileWithTagCount>>()
             })
             .map(|files| sort(files, req.sort.unwrap_or(NameAsc)))
             .inspect(|files| debug!("Found {:?} files", files.len()))
@@ -106,7 +113,7 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
     if let Ok(files) = file_system.get_files_for_path(search_path) {
         debug!("Valid search path: {:?}", search_path);
 
-        let mut photos = files
+        let photos = files
             .iter()
             .filter(|&f| {
                 f.metadata().map_or_else(
@@ -122,8 +129,14 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
                 relative.to_path_buf()
             })
             .map(|f| f.to_str().unwrap().to_string())
-            .filter(|file_path| {
+            .map(|file_name| {
-                if let (Some(tag_ids), Ok(mut tag_dao)) = (&req.tag_ids, tag_dao.lock()) {
+                let mut tag_dao = tag_dao.lock().expect("Unable to get TagDao");
+                let file_tags = tag_dao.get_tags_for_path(&file_name).unwrap_or_default();
+
+                (file_name, file_tags)
+            })
+            .filter(|(_, file_tags)| {
+                if let Some(tag_ids) = &req.tag_ids {
                     let tag_ids = tag_ids
                         .split(',')
                         .filter_map(|t| t.parse().ok())
@@ -138,7 +151,6 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
                         .collect::<Vec<i32>>();
 
                     let filter_mode = &req.tag_filter_mode.unwrap_or(FilterMode::Any);
-                    let file_tags = tag_dao.get_tags_for_path(file_path).unwrap_or_default();
                     let excluded = file_tags.iter().any(|t| excluded_tag_ids.contains(&t.id));
 
                     return !excluded
@@ -152,11 +164,20 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
 
                 true
             })
-            .collect::<Vec<String>>();
+            .map(|(file_name, tags)| FileWithTagCount {
+                file_name,
+                tag_count: tags.len() as i64,
+            })
+            .collect::<Vec<FileWithTagCount>>();
 
+        let mut response_files = photos
+            .clone()
+            .into_iter()
+            .map(|f| f.file_name)
+            .collect::<Vec<String>>();
         if let Some(sort_type) = req.sort {
             debug!("Sorting files: {:?}", sort_type);
-            photos = sort(photos, sort_type)
+            response_files = sort(photos, sort_type)
         }
 
         let dirs = files
@@ -169,25 +190,37 @@ pub async fn list_photos<TagD: TagDao, FS: FileSystemAccess>(
             .map(|f| f.to_str().unwrap().to_string())
             .collect::<Vec<String>>();
 
-        HttpResponse::Ok().json(PhotosResponse { photos, dirs })
+        HttpResponse::Ok().json(PhotosResponse {
+            photos: response_files,
+            dirs,
+        })
     } else {
         error!("Bad photos request: {}", req.path);
         HttpResponse::BadRequest().finish()
     }
 }
 
-fn sort(mut files: Vec<String>, sort_type: SortType) -> Vec<String> {
+fn sort(mut files: Vec<FileWithTagCount>, sort_type: SortType) -> Vec<String> {
     match sort_type {
         SortType::Shuffle => files.shuffle(&mut thread_rng()),
         SortType::NameAsc => {
-            files.sort();
+            files.sort_by(|l, r| l.file_name.cmp(&r.file_name));
         }
         SortType::NameDesc => {
-            files.sort_by(|l, r| r.cmp(l));
+            files.sort_by(|l, r| r.file_name.cmp(&l.file_name));
+        }
+        SortType::TagCountAsc => {
+            files.sort_by(|l, r| l.tag_count.cmp(&r.tag_count));
+        }
+        SortType::TagCountDesc => {
+            files.sort_by(|l, r| r.tag_count.cmp(&l.tag_count));
         }
     }
 
     files
+        .iter()
+        .map(|f| f.file_name.clone())
+        .collect::<Vec<String>>()
 }
 
 pub fn list_files(dir: &Path) -> io::Result<Vec<PathBuf>> {
@@ -396,10 +429,7 @@ mod tests {
             if self.err {
                 Err(anyhow!("Error for test"))
             } else if let Some(files) = self.files.get(path) {
-                Ok(files
+                Ok(files.iter().map(PathBuf::from).collect::<Vec<PathBuf>>())
-                    .iter()
-                    .map(PathBuf::from)
-                    .collect::<Vec<PathBuf>>())
             } else {
                 Ok(Vec::new())
             }

src/main.rs

@@ -184,8 +184,10 @@ async fn upload_image(
             &full_path.to_str().unwrap().to_string(),
             true,
         ) {
-            let context = opentelemetry::Context::new().with_remote_span_context(span.span_context().clone());
+            let context =
-            tracer.span_builder("file write")
+                opentelemetry::Context::new().with_remote_span_context(span.span_context().clone());
+            tracer
+                .span_builder("file write")
                 .start_with_context(&tracer, &context);
 
             if !full_path.is_file() && is_image_or_video(&full_path) {
@@ -322,7 +324,10 @@ async fn get_video_part(
         file.into_response(&request)
     } else {
         error!("Video part not found: {:?}", file_part);
-        span.set_status(Status::error(format!("Video part not found '{}'", file_part.to_str().unwrap())));
+        span.set_status(Status::error(format!(
+            "Video part not found '{}'",
+            file_part.to_str().unwrap()
+        )));
         HttpResponse::NotFound().finish()
     }
 }
@@ -454,7 +459,8 @@ fn create_thumbnails() {
 
                 let mut video_span = tracer.start_with_context(
                     "generate_video_thumbnail",
-                    &opentelemetry::Context::new().with_remote_span_context(span.span_context().clone()),
+                    &opentelemetry::Context::new()
+                        .with_remote_span_context(span.span_context().clone()),
                 );
                 video_span.set_attributes(vec![
                     KeyValue::new("type", "video"),
@@ -538,7 +544,6 @@ fn main() -> std::io::Result<()> {
     if let Err(err) = dotenv::dotenv() {
         println!("Error parsing .env {:?}", err);
     }
-    // env_logger::init();
 
     run_migrations(&mut connect()).expect("Failed to run migrations");
 
@@ -546,9 +551,17 @@ fn main() -> std::io::Result<()> {
 
     let system = actix::System::new();
     system.block_on(async {
-
+        // Just use basic logger when running a non-release build
+        #[cfg(debug_assertions)]
+        {
+            env_logger::init();
+        }
+        #[cfg(not(debug_assertions))]
+        {
             otel::init_logs();
             otel::init_tracing();
+        }
+
         create_thumbnails();
 
         let app_data = Data::new(AppState::default());

src/tags.rs

@@ -202,12 +202,12 @@ pub trait TagDao {
         &mut self,
         tag_ids: Vec<i32>,
         exclude_tag_ids: Vec<i32>,
-    ) -> anyhow::Result<Vec<String>>;
+    ) -> anyhow::Result<Vec<FileWithTagCount>>;
     fn get_files_with_any_tag_ids(
         &mut self,
         tag_ids: Vec<i32>,
         exclude_tag_ids: Vec<i32>,
-    ) -> anyhow::Result<Vec<String>>;
+    ) -> anyhow::Result<Vec<FileWithTagCount>>;
 }
 
 pub struct SqliteTagDao {
@@ -277,7 +277,7 @@ impl TagDao for SqliteTagDao {
             .and_then(|_| {
                 info!("Inserted tag: {:?}", name);
                 define_sql_function! {
-                    fn last_insert_rowid() -> diesel::sql_types::Integer;
+                    fn last_insert_rowid() -> Integer;
                 }
                 diesel::select(last_insert_rowid())
                     .get_result::<i32>(&mut self.connection)
@@ -353,7 +353,7 @@ impl TagDao for SqliteTagDao {
         &mut self,
         tag_ids: Vec<i32>,
         exclude_tag_ids: Vec<i32>,
-    ) -> anyhow::Result<Vec<String>> {
+    ) -> anyhow::Result<Vec<FileWithTagCount>> {
         use diesel::dsl::*;
 
         let exclude_subquery = tagged_photo::table
@@ -365,10 +365,21 @@ impl TagDao for SqliteTagDao {
             .filter(tagged_photo::tag_id.eq_any(tag_ids.clone()))
             .filter(tagged_photo::photo_name.ne_all(exclude_subquery))
             .group_by(tagged_photo::photo_name)
-            .select((tagged_photo::photo_name, count(tagged_photo::tag_id)))
+            .select((
+                tagged_photo::photo_name,
+                count_distinct(tagged_photo::tag_id),
+            ))
             .having(count_distinct(tagged_photo::tag_id).ge(tag_ids.len() as i64))
-            .select(tagged_photo::photo_name)
+            .get_results::<(String, i64)>(&mut self.connection)
-            .get_results::<String>(&mut self.connection)
+            .map(|results| {
+                results
+                    .into_iter()
+                    .map(|(file_name, tag_count)| FileWithTagCount {
+                        file_name,
+                        tag_count,
+                    })
+                    .collect()
+            })
             .with_context(|| format!("Unable to get Tagged photos with ids: {:?}", tag_ids))
     }
 
@@ -376,7 +387,7 @@ impl TagDao for SqliteTagDao {
         &mut self,
         tag_ids: Vec<i32>,
         exclude_tag_ids: Vec<i32>,
-    ) -> anyhow::Result<Vec<String>> {
+    ) -> anyhow::Result<Vec<FileWithTagCount>> {
         use diesel::dsl::*;
 
         let exclude_subquery = tagged_photo::table
@@ -388,9 +399,20 @@ impl TagDao for SqliteTagDao {
             .filter(tagged_photo::tag_id.eq_any(tag_ids.clone()))
             .filter(tagged_photo::photo_name.ne_all(exclude_subquery))
             .group_by(tagged_photo::photo_name)
-            .select((tagged_photo::photo_name, count(tagged_photo::tag_id)))
+            .select((
-            .select(tagged_photo::photo_name)
+                tagged_photo::photo_name,
-            .get_results::<String>(&mut self.connection)
+                count_distinct(tagged_photo::tag_id),
+            ))
+            .get_results::<(String, i64)>(&mut self.connection)
+            .map(|results| {
+                results
+                    .into_iter()
+                    .map(|(file_name, tag_count)| FileWithTagCount {
+                        file_name,
+                        tag_count,
+                    })
+                    .collect()
+            })
             .with_context(|| format!("Unable to get Tagged photos with ids: {:?}", tag_ids))
     }
 }
@@ -517,7 +539,7 @@ mod tests {
             &mut self,
             tag_ids: Vec<i32>,
             _exclude_tag_ids: Vec<i32>,
-        ) -> anyhow::Result<Vec<String>> {
+        ) -> anyhow::Result<Vec<FileWithTagCount>> {
             todo!()
         }
 
@@ -525,7 +547,7 @@ mod tests {
             &mut self,
             _tag_ids: Vec<i32>,
             _exclude_tag_ids: Vec<i32>,
-        ) -> anyhow::Result<Vec<String>> {
+        ) -> anyhow::Result<Vec<FileWithTagCount>> {
             todo!()
         }
     }
@@ -607,3 +629,9 @@ mod tests {
         );
     }
 }
+
+#[derive(Debug, Clone)]
+pub struct FileWithTagCount {
+    pub file_name: String,
+    pub tag_count: i64,
+}

src/testhelpers.rs

@@ -48,10 +48,7 @@ impl UserDao for TestUserDao {
     }
 
     fn user_exists(&mut self, user: &str) -> bool {
-        self.user_map
+        self.user_map.borrow().iter().any(|u| u.username == user)
-            .borrow()
-            .iter()
-            .any(|u| u.username == user)
     }
 }