Avoid redundant image assignment, propagate missing image errors, refactor

src/build.rs

@@ -30,9 +30,9 @@ pub enum Breadcrumb {
 impl Breadcrumb {
     pub fn label(&self) -> &str {
         match self {
-            Breadcrumb::Collection(label) => &label,
-            Breadcrumb::Playlist(label) => &label,
-            Breadcrumb::Video(label) => &label
+            Breadcrumb::Collection(label) => label,
+            Breadcrumb::Playlist(label) => label,
+            Breadcrumb::Video(label) => label
         }
     }
 }
@@ -57,16 +57,16 @@ pub fn perform_build(context: &Context) {
         include_str!("build/assets/scripts.js")
     ).unwrap();
 
-    styles::generate(&context, &site);
+    styles::generate(context, &site);
 }
 
 fn build_collection(
     context: &Context,
     collection: &Collection,
     site: &Site,
-    breadcrumbs: &Vec<Breadcrumb>
+    breadcrumbs: &[Breadcrumb]
 ) {
-    let mut breadcrumbs_extended = breadcrumbs.clone();
+    let mut breadcrumbs_extended = breadcrumbs.to_owned();
 
     breadcrumbs_extended.push(Breadcrumb::Collection(collection.label().to_string()));
 
@@ -89,9 +89,9 @@ fn build_playlist(
     context: &Context,
     playlist: &Playlist,
     site: &Site,
-    breadcrumbs: &Vec<Breadcrumb>
+    breadcrumbs: &[Breadcrumb]
 ) {
-    let mut breadcrumbs_extended = breadcrumbs.clone();
+    let mut breadcrumbs_extended = breadcrumbs.to_owned();
 
     breadcrumbs_extended.push(Breadcrumb::Playlist(playlist.label().to_string()));
 
@@ -125,9 +125,9 @@ fn build_video(
     context: &Context,
     video: &Video,
     site: &Site,
-    breadcrumbs: &Vec<Breadcrumb>
+    breadcrumbs: &[Breadcrumb]
 ) {
-    let mut breadcrumbs_extended = breadcrumbs.clone();
+    let mut breadcrumbs_extended = breadcrumbs.to_owned();
 
     breadcrumbs_extended.push(Breadcrumb::Video(video.label().to_string()));
 

src/build/render.rs

@@ -1,9 +1,6 @@
 use indoc::formatdoc;
-use std::fs;
-use std::path::Path;
-use std::sync::Arc;
 
-use crate::{Playlist, Site, Video, VideoFile};
+use crate::{Site, Video, VideoFile};
 use crate::build::Breadcrumb;
 use crate::util::{casual_duration, human_size};
 
@@ -19,7 +16,7 @@ pub fn layout(
     site: &Site,
     body: &str,
     title: &str,
-    breadcrumbs: &Vec<Breadcrumb>
+    breadcrumbs: &[Breadcrumb]
 ) -> String {
     // TODO: Hash assets (cache expiration)
     let head = formatdoc!(r#"

src/collection.rs

@@ -361,7 +361,7 @@ impl Collection {
         }
 
         if !hyper_dir.image_files.is_empty() {
-            let mut error = "Image files in a collection directory are not supported/used, maybe you wanted to put them in subfolder instead?".to_string();
+            let mut error = "Image files in a collection directory are not supported/used, maybe you wanted to put them in a subfolder instead?".to_string();
 
             for image_file in &hyper_dir.image_files {
                 error.push_str(&format!("\n- {}", image_file.display()));
@@ -371,7 +371,7 @@ impl Collection {
         }
 
         if !hyper_dir.video_files.is_empty() {
-            let mut error = "No video files should be directly in a collection directory, maybe you wanted to put them in subfolder instead?".to_string();
+            let mut error = "No video files should be directly in a collection directory, maybe you wanted to put them in a subfolder instead?".to_string();
 
             for video_file in &hyper_dir.video_files {
                 error.push_str(&format!("\n- {}", video_file.display()));

src/editor/endpoints/collection.rs

@@ -67,7 +67,7 @@ pub async fn create_playlist(
     let slug_error = if let Err(message) = validate_slug(slug_trimmed) {
         Some(message)
     } else if context.site_dir.join(&path.path).join(slug_trimmed).exists() {
-        Some(format!("Already used by another collection, playlist or video"))
+        Some("Already used by another collection, playlist or video".to_string())
     } else {
         None
     };
@@ -118,7 +118,7 @@ pub async fn create_subcollection(
     let slug_error = if let Err(message) = validate_slug(slug_trimmed) {
         Some(message)
     } else if context.site_dir.join(&path.path).join(slug_trimmed).exists() {
-        Some(format!("Already used by another collection, playlist or video"))
+        Some("Already used by another collection, playlist or video".to_string())
     } else {
         None
     };
@@ -258,7 +258,7 @@ pub fn edit_page(
         Some(CollectionFormFeedback::Site(site_form_feedback)) => Some(site_form_feedback),
         _ => None
     };
-    let site_edit = endpoints::site::edit(&context, site_form_feedback, path);
+    let site_edit = endpoints::site::edit(context, site_form_feedback, path);
 
     let (input_slug_create_playlist, feedback_create_playlist) = if let Some(CollectionFormFeedback::CreatePlaylist { slug }) = &form_feedback {
         (
@@ -356,7 +356,7 @@ pub async fn edit(
 ) -> HttpResponse {
     match context.get_collection(&path.path) {
         Some(collection) => edit_page(&collection, &context, None),
-        None => return not_found(&context, &format!("Collection {} not found.", &path.path))
+        None => not_found(&context, &format!("Collection {} not found.", &path.path))
     }
 }
 
@@ -392,7 +392,8 @@ pub async fn r#move(
         collection.persist_in_site(&context.site_dir);
 
         let redirect_url = format!("/collection/{}", &collection.path);
-        return Either::Left(Redirect::to(redirect_url).see_other())
+
+        Either::Left(Redirect::to(redirect_url).see_other())
     } else {
         let collection = match context.get_collection(&path.path) {
             Some(collection) => collection,
@@ -405,9 +406,7 @@ pub async fn r#move(
             slug: Field::validated(slug_trimmed, slug_error)
         };
 
-        Either::Right(
-            edit_page(&collection, &context, Some(form_feedback))
-        )
+        Either::Right(edit_page(&collection, &context, Some(form_feedback)))
     }
 }
 
@@ -438,7 +437,8 @@ pub async fn update(
         collection.persist_in_site(&context.site_dir);
 
         let redirect_url = format!("/collection/{}", &collection.path);
-        return Either::Left(Redirect::to(redirect_url).see_other())
+
+        Either::Left(Redirect::to(redirect_url).see_other())
     } else {
         let collection = match context.get_collection(&path.path) {
             Some(collection) => collection,
@@ -453,12 +453,6 @@ pub async fn update(
             unlisted
         };
 
-        return Either::Right(
-            edit_page(
-                &collection,
-                &context,
-                Some(form_feedback)
-            )
-        )
+        Either::Right(edit_page(&collection, &context, Some(form_feedback)))
     }
 }

src/editor/endpoints/playlist.rs

@@ -183,7 +183,7 @@ pub fn edit_page(
         Some(PlaylistFormFeedback::Site(site_form_feedback)) => Some(site_form_feedback),
         _ => None
     };
-    let site_edit = endpoints::site::edit(&context, site_form_feedback, path);
+    let site_edit = endpoints::site::edit(context, site_form_feedback, path);
 
     let body = formatdoc!(r#"
         {errors}

src/editor/endpoints/site.rs

@@ -132,7 +132,7 @@ fn edit_page(
         (
             if theme.key == "dark" { "checked " } else { "" },
             if theme.key == "light" { "checked " } else { "" },
-            form_field(&base_url, "base_url", "Base URL", "Base URL"),
+            form_field(base_url, "base_url", "Base URL", "Base URL"),
             r#"<span class="feedback">Not Saved</span>"#
         )
     } else {
@@ -248,15 +248,15 @@ pub async fn update(
         let http_response = match &site.content {
             SiteContent::Collection(collection) => {
                 let form_feedback = endpoints::collection::CollectionFormFeedback::Site(site_form_feedback);
-                endpoints::collection::edit_page(&collection, &context, Some(form_feedback))
+                endpoints::collection::edit_page(collection, &context, Some(form_feedback))
             }
             SiteContent::Playlist(playlist) => {
                 let form_feedback = endpoints::playlist::PlaylistFormFeedback::Site(site_form_feedback);
-                endpoints::playlist::edit_page(&context, Some(form_feedback), &playlist)
+                endpoints::playlist::edit_page(&context, Some(form_feedback), playlist)
             }
             SiteContent::Video(video) => {
                 let form_feedback = endpoints::video::VideoFormFeedback::Site(site_form_feedback);
-                endpoints::video::edit_page(&context, Some(form_feedback), &video)
+                endpoints::video::edit_page(&context, Some(form_feedback), video)
             }
             // TODO: At best a time of check/time of use edge case possibility?
             _ => unreachable!()

src/editor/endpoints/video.rs

@@ -199,7 +199,7 @@ pub fn edit_page(
                         let browser_support = match video_meta.browser_support.analysis() {
                             Some(Ok(message)) => format!("[Very likely playable in browsers: {message}]"),
                             Some(Err(message)) => format!("[Unfit for browser playback: {message}]"),
-                            None => format!("[Might be incompatible with and not play in people's browsers]"),
+                            None => "[Might be incompatible with and not play in people's browsers]".to_string(),
                         };
 
                         format!("{width}×{height} {duration} {size} {browser_support}")
@@ -313,7 +313,7 @@ pub fn edit_page(
         Some(VideoFormFeedback::Site(site_form_feedback)) => Some(site_form_feedback),
         _ => None
     };
-    let site_edit = endpoints::site::edit(&context, site_form_feedback, &video.path);
+    let site_edit = endpoints::site::edit(context, site_form_feedback, &video.path);
 
     let upload_url = format!("/upload-video/{path}");
     let video_upload_button = upload_widget(&upload_url, "Upload New Version");
@@ -720,5 +720,5 @@ pub async fn upload_video(
     }
 
     let http_response = not_found(&context, "No actionable payload found in request");
-    return Ok(Either::Right(http_response));
+    Ok(Either::Right(http_response))
 }

src/editor/widgets.rs

@@ -38,7 +38,7 @@ fn collection_tree(
     collection: &Collection,
     path: &str
 ) -> String {
-    let mut subcollections_sorted: Vec<Arc<Collection>> = collection.subcollections.iter().cloned().collect();
+    let mut subcollections_sorted: Vec<Arc<Collection>> = collection.subcollections.to_vec();
 
     subcollections_sorted.sort_unstable_by(|a, b| {
         let a_criteria = a.title.as_deref().unwrap_or_else(|| a.slug());
@@ -75,7 +75,7 @@ fn collection_tree(
         .collect::<Vec<String>>()
         .join("");
 
-    let mut playlists_sorted: Vec<Arc<Playlist>> = collection.playlists.iter().cloned().collect();
+    let mut playlists_sorted: Vec<Arc<Playlist>> = collection.playlists.to_vec();
 
     playlists_sorted.sort_unstable_by(|a, b| {
         let a_criteria = a.title.as_deref().unwrap_or_else(|| a.slug());
@@ -143,7 +143,7 @@ fn collection_tree(
         .collect::<Vec<String>>()
         .join("");
 
-    let mut videos_sorted: Vec<Arc<Video>> = collection.videos.iter().cloned().collect();
+    let mut videos_sorted: Vec<Arc<Video>> = collection.videos.to_vec();
 
     videos_sorted.sort_unstable_by(|a, b| {
         let a_criteria = a.title.as_deref().unwrap_or_else(|| a.slug());
@@ -342,7 +342,7 @@ pub fn not_found(context: &Context, message: &str) -> HttpResponse {
     "#);
 
     let site = &context.get_site();
-    let html = layout(context, "", &site, &body, "Not Found");
+    let html = layout(context, "", site, &body, "Not Found");
 
     HttpResponse::Ok()
         .content_type(ContentType::html())
@@ -390,7 +390,7 @@ pub fn site_tree(path: &str, site: &Site, site_dir: &Path) -> (String, String) {
                     </li>
                 "#)
             } else {
-                let mut videos_sorted: Vec<Arc<Video>> = playlist.videos_physical.iter().cloned().collect();
+                let mut videos_sorted: Vec<Arc<Video>> = playlist.videos_physical.to_vec();
 
                 videos_sorted.sort_unstable_by(|a, b| {
                     let a_criteria = a.title.as_deref().unwrap_or_else(|| a.slug());
@@ -456,7 +456,7 @@ pub fn site_tree(path: &str, site: &Site, site_dir: &Path) -> (String, String) {
 
 fn tree_node(
     active: bool,
-    errors: &Vec<String>,
+    errors: &[String],
     href: &str,
     icon: &str,
     slug: &str,

src/image_processor.rs

@@ -54,7 +54,7 @@ impl ImageProcessor {
             let resized_image = cropped_image.resize(new_width, new_height, FilterType::Lanczos3);
             self.export(context, &resized_image)
         } else {
-            self.export(context, &cropped_image)
+            self.export(context, cropped_image)
         }
     }
 

src/playlist.rs

@@ -26,16 +26,24 @@ pub struct Playlist {
     /// one needs to know the link to access them.
     pub unlisted: bool,
 	pub videos_physical: Vec<Arc<Video>>,
+    // TODO: We could potentially drop this and follow up with the plan of
+    //       making video order simply a sorting matter, i.e. based on video release_date
+    //       and/or a user-assigned video "number" respectively.
     pub videos_virtual: Vec<String>
 }
 
 impl Playlist {
-    /// TODO:
-    /// - poster support (image description/path)
-    /// - video listing
     fn apply_manifest(&mut self, manifest_path: &Path) {
         let content = fs::read_to_string(manifest_path).unwrap();
 
+        // TODO: Tricky manifest problem/question in general: If there is anything wrong
+        //       with the manifest (especially: syntax error rendering the entire manifest useless),
+        //       we can potentially erase everything contained in it by doing an update via the browser
+        //       editor (it persists what has been read - nothing!). Consequently we should e.g. disable
+        //       the edit form/updates via browser editor while there are issues with the manifest, or
+        //       try to read in some things in a "broken state", e.g. registering a poster although the
+        //       image referenced was not found.
+
         let document = match enolib::parse(&content) {
             Ok(document) => document,
             Err(err) => {
@@ -67,7 +75,7 @@ impl Playlist {
                             let poster_file = PosterFile::new(None, file_meta, value);
                             self.poster = Some(poster_file);
                         } else {
-                            // TODO: Error
+                            self.errors.push(format!("Error in {} (Path to poster is invalid, file not found)", manifest_path.display()));
                         }
                     }
                     Err(err) => self.errors.push(format!("Error in {}:{} ({})", manifest_path.display(), err.line, err))
@@ -211,11 +219,11 @@ impl Playlist {
             playlist.read_subdir(context, &hyper_subdir);
         }
 
-        if !hyper_dir.image_files.is_empty() {
-            // TODO: Some strategy what we do with multiple images (e.g. heuristic to pick most probable poster, use additional ones, etc.)
+        // TODO: Some strategy what we do with multiple images (e.g. heuristic to pick most probable poster, use additional ones, etc.)
+        if !hyper_dir.image_files.is_empty() && playlist.poster.is_none() {
             let first_image = hyper_dir.image_files.first().unwrap();
 
-            let file_meta = FileMeta::new(&first_image);
+            let file_meta = FileMeta::new(first_image);
             let file_name = first_image.file_name().unwrap().to_string_lossy().to_string();
 
             let poster_path = format!("{}/{}", &playlist.path, file_name);

src/video.rs

@@ -116,7 +116,7 @@ impl Video {
                             let poster_file = PosterFile::new(None, file_meta, value);
                             self.poster = Some(poster_file);
                         } else {
-                            // TODO: Error
+                            self.errors.push(format!("Error in {} (Path to poster is invalid, file not found)", manifest_path.display()));
                         }
                     }
                     Err(err) => self.errors.push(format!("Error in {}:{} ({})", manifest_path.display(), err.line, err))
@@ -258,11 +258,11 @@ impl Video {
             video.errors.push(format!("Ignoring folders {folder_names} inside video folder {} (only files may be placed inside a video folder).", hyper_dir.path.display()));
         }
 
-        if !hyper_dir.image_files.is_empty() {
-            // TODO: Some strategy what we do with multiple images (e.g. heuristic to pick most probable poster, use additional ones, etc.)
+        // TODO: Some strategy what we do with multiple images (e.g. heuristic to pick most probable poster, use additional ones, etc.)
+        if !hyper_dir.image_files.is_empty() && video.poster.is_none() {
             let first_image = hyper_dir.image_files.first().unwrap();
 
-            let file_meta = FileMeta::new(&first_image);
+            let file_meta = FileMeta::new(first_image);
             let file_name = first_image.file_name().unwrap().to_string_lossy().to_string();
 
             let poster_path = format!("{}/{}", &video.path, file_name);
@@ -277,7 +277,7 @@ impl Video {
         }
 
         for video_file in &hyper_dir.video_files {
-            let file_meta = FileMeta::new(&video_file);
+            let file_meta = FileMeta::new(video_file);
             let file_name = video_file.file_name().unwrap().to_string_lossy().to_string();
 
             let version_path = format!("{}/{}", &video.path, file_name);
@@ -342,17 +342,6 @@ impl Video {
             let video_dir = context.site_dir.join(&self.path);
             fs::remove_dir_all(video_dir).unwrap();
         }
-
-        // TODO: (Potentially) remove cached assets such as resized images, cached metadata etc.
-        // let filename_in_cache = format!("{}.bincode", &self.id);
-        // let path_in_cache = context.cache_dir.join(filename_in_cache);
-        
-        // fs::remove_file(path_in_cache).unwrap();
-
-        // if let Some(poster) = &self.assets.poster {
-        //     let poster_path = context.cache_dir.join(poster);
-        //     fs::remove_file(poster_path).unwrap();
-        // }
     }
 
     pub fn write_manifest(&self, context: &Context) {

src/workers.rs

@@ -61,7 +61,7 @@ fn compute_metadata(context: &Context, video: Arc<Video>) -> Arc<Video> {
 }
 
 fn compute_poster(context: &Context, video: Arc<Video>) -> Arc<Video> {
-    if let Ok(file_name) = transcode::compute_poster(&context, &video) {
+    if let Ok(file_name) = transcode::compute_poster(context, &video) {
         let file_path = context.site_dir.join(&video.path).join(&file_name);
         let file_meta = FileMeta::new(&file_path);
         let poster_file = PosterFile::new(None, file_meta, file_name);
@@ -71,7 +71,7 @@ fn compute_poster(context: &Context, video: Arc<Video>) -> Arc<Video> {
         };
 
         if let Ok(video_updated) = context.update_video(mutation, &video.path) {
-            video_updated.write_manifest(&context);
+            video_updated.write_manifest(context);
             return video_updated;
         }
     }
@@ -148,7 +148,7 @@ fn compute_poster_assets(context: &Context, video: Arc<Video>) {
         };
 
         if let Ok(video) = context.update_video(mutation, &video.path) {
-            video.write_manifest(&context);
+            video.write_manifest(context);
         }
     } 
 }
@@ -206,8 +206,7 @@ pub fn start_threads(count: usize, context_ref: &Arc<Context>) {
                                 video.persist_in_cache(&context.cache_dir);
                             }
                         }
-                        // TODO: Temporary hack - for now we don't set video_metadata in transcode::encode (although we totally could, see TODO in there)
-                        //       Instead of appending a FastTasks job, fill video metadata in transcode::encode and remove this job creation here.
+
                         let follow_up_job = Job::new(JobKind::FastTasks, Arc::clone(&job.video));
                         context.job_queue.lock().unwrap().push(follow_up_job);