From 770277907b9ee9f6a0d3d429fdc30f641b112123 Mon Sep 17 00:00:00 2001 From: Simon Repp Date: Sat, 16 Mar 2024 12:48:50 +0100 Subject: [PATCH] Avoid redundant image assignment, propagate missing image errors, refactor --- src/build.rs | 20 ++++++++++---------- src/build/render.rs | 7 ++----- src/collection.rs | 4 ++-- src/editor/endpoints/collection.rs | 26 ++++++++++---------------- src/editor/endpoints/playlist.rs | 2 +- src/editor/endpoints/site.rs | 8 ++++---- src/editor/endpoints/video.rs | 6 +++--- src/editor/widgets.rs | 12 ++++++------ src/image_processor.rs | 2 +- src/playlist.rs | 22 +++++++++++++++------- src/video.rs | 21 +++++---------------- src/workers.rs | 9 ++++----- 12 files changed, 63 insertions(+), 76 deletions(-) diff --git a/src/build.rs b/src/build.rs index b38f857..f5c8a64 100644 --- a/src/build.rs +++ b/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 + 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 + 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 + breadcrumbs: &[Breadcrumb] ) { - let mut breadcrumbs_extended = breadcrumbs.clone(); + let mut breadcrumbs_extended = breadcrumbs.to_owned(); breadcrumbs_extended.push(Breadcrumb::Video(video.label().to_string())); diff --git a/src/build/render.rs b/src/build/render.rs index 2183fcb..d594c36 100644 --- a/src/build/render.rs +++ b/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 + breadcrumbs: &[Breadcrumb] ) -> String { // TODO: Hash assets (cache expiration) let head = formatdoc!(r#" diff --git a/src/collection.rs b/src/collection.rs index 3da60d4..bf31d7c 100644 --- a/src/collection.rs +++ b/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())); diff --git a/src/editor/endpoints/collection.rs b/src/editor/endpoints/collection.rs index 9c446c2..82aca30 100644 --- a/src/editor/endpoints/collection.rs +++ b/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))) } } \ No newline at end of file diff --git a/src/editor/endpoints/playlist.rs b/src/editor/endpoints/playlist.rs index 132b06a..6373f05 100644 --- a/src/editor/endpoints/playlist.rs +++ b/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} diff --git a/src/editor/endpoints/site.rs b/src/editor/endpoints/site.rs index 3b519c8..dff030a 100644 --- a/src/editor/endpoints/site.rs +++ b/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#""# ) } 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!() diff --git a/src/editor/endpoints/video.rs b/src/editor/endpoints/video.rs index 12de982..9a8a827 100644 --- a/src/editor/endpoints/video.rs +++ b/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)) } \ No newline at end of file diff --git a/src/editor/widgets.rs b/src/editor/widgets.rs index 60eda45..0f51e96 100644 --- a/src/editor/widgets.rs +++ b/src/editor/widgets.rs @@ -38,7 +38,7 @@ fn collection_tree( collection: &Collection, path: &str ) -> String { - let mut subcollections_sorted: Vec> = collection.subcollections.iter().cloned().collect(); + let mut subcollections_sorted: Vec> = 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::>() .join(""); - let mut playlists_sorted: Vec> = collection.playlists.iter().cloned().collect(); + let mut playlists_sorted: Vec> = 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::>() .join(""); - let mut videos_sorted: Vec> = collection.videos.iter().cloned().collect(); + let mut videos_sorted: Vec> = 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) { "#) } else { - let mut videos_sorted: Vec> = playlist.videos_physical.iter().cloned().collect(); + let mut videos_sorted: Vec> = 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, + errors: &[String], href: &str, icon: &str, slug: &str, diff --git a/src/image_processor.rs b/src/image_processor.rs index 5530d4e..b17aeec 100644 --- a/src/image_processor.rs +++ b/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) } } diff --git a/src/playlist.rs b/src/playlist.rs index 58e79c2..1bbe841 100644 --- a/src/playlist.rs +++ b/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>, + // 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 } 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); diff --git a/src/video.rs b/src/video.rs index 04a2706..48d16e2 100644 --- a/src/video.rs +++ b/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) { diff --git a/src/workers.rs b/src/workers.rs index 52e99df..14cb37b 100644 --- a/src/workers.rs +++ b/src/workers.rs @@ -61,7 +61,7 @@ fn compute_metadata(context: &Context, video: Arc