From 73c90a7c1179860f19fb1a3642d3c1a3a49e0cbb Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 7 Jan 2024 15:48:40 +0100 Subject: [PATCH 01/16] Frist draft of mh-edit binary --- Cargo.toml | 9 ++- src/bin/mh-edit.rs | 181 +++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 163 ++++++++++++++++++++++++++++++++++++++-- 3 files changed, 346 insertions(+), 7 deletions(-) create mode 100644 src/bin/mh-edit.rs diff --git a/Cargo.toml b/Cargo.toml index 33dc5d7..395c326 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,14 +23,19 @@ tempfile = "3.5.0" [features] default = ["database-json", "library-beets"] +bin = ["structopt"] database-json = ["serde_json"] library-beets = [] ssh-library = ["openssh", "tokio"] -tui = ["structopt", "crossterm", "ratatui"] +tui = ["crossterm", "ratatui"] [[bin]] name = "musichoard" -required-features = ["database-json", "library-beets", "ssh-library", "tui"] +required-features = ["bin", "database-json", "library-beets", "ssh-library", "tui"] + +[[bin]] +name = "mh-edit" +required-features = ["bin", "database-json"] [package.metadata.docs.rs] all-features = true diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs new file mode 100644 index 0000000..58493c5 --- /dev/null +++ b/src/bin/mh-edit.rs @@ -0,0 +1,181 @@ +use std::fs::OpenOptions; +use std::path::PathBuf; +use structopt::StructOpt; + +use musichoard::{ + database::{ + json::{backend::JsonDatabaseFileBackend, JsonDatabase}, + IDatabase, + }, + library::NoLibrary, + Artist, ArtistId, ArtistProperties, MusicHoard, +}; + +#[derive(StructOpt, Debug)] +struct Opt { + #[structopt(subcommand)] + category: Category, + + #[structopt( + long = "database", + name = "database file path", + default_value = "database.json" + )] + database_file_path: PathBuf, +} + +#[derive(StructOpt, Debug)] +enum Category { + Artist(ArtistCommand), +} + +#[derive(StructOpt, Debug)] +enum ArtistCommand { + New(ArtistValue), + Delete(ArtistValue), + #[structopt(name = "musicbrainz")] + MusicBrainz(UrlCommand), + #[structopt(name = "musicbutler")] + MusicButler(UrlCommand), + Bandcamp(UrlCommand), + Qobuz(UrlCommand), +} + +#[derive(StructOpt, Debug)] +struct ArtistValue { + artist: String, +} + +#[derive(StructOpt, Debug)] +enum UrlCommand { + Add(T), + Remove(T), + Set(T), + Clear(ArtistValue), +} + +#[derive(StructOpt, Debug)] +struct SingleUrlValue { + artist: String, + url: String, +} + +#[derive(StructOpt, Debug)] +struct MultiUrlValue { + artist: String, + urls: Vec, +} + +fn main() { + let opt = Opt::from_args(); + + let lib: Option = None; + let db = Some(JsonDatabase::new(JsonDatabaseFileBackend::new( + &opt.database_file_path, + ))); + + let mut music_hoard = MusicHoard::new(lib, db); + music_hoard + .load_from_database() + .expect("failed to load database"); + + match opt.category { + Category::Artist(artist_command) => { + match artist_command { + ArtistCommand::New(artist_value) => { + music_hoard + .new_artist(ArtistId::new(artist_value.artist)) + .expect("failed to add new artist"); + } + ArtistCommand::Delete(artist_value) => { + music_hoard + .delete_artist(ArtistId::new(artist_value.artist)) + .expect("failed to delete artist"); + } + ArtistCommand::MusicBrainz(url_command) => match url_command { + UrlCommand::Add(single_url_value) => { + music_hoard + .add_musicbrainz_url( + ArtistId::new(single_url_value.artist), + single_url_value.url, + ) + .expect("failed to add MusicBrainz URL"); + } + UrlCommand::Remove(single_url_value) => { + music_hoard + .remove_musicbrainz_url( + ArtistId::new(single_url_value.artist), + single_url_value.url, + ) + .expect("failed to remove MusicBrainz URL"); + } + UrlCommand::Set(single_url_value) => { + music_hoard + .set_musicbrainz_url( + ArtistId::new(single_url_value.artist), + single_url_value.url, + ) + .expect("failed to set MusicBrainz URL"); + } + UrlCommand::Clear(artist_value) => { + music_hoard + .clear_musicbrainz_url(ArtistId::new(artist_value.artist)) + .expect("failed to clear MusicBrainz URL"); + } + }, + ArtistCommand::MusicButler(url_command) => { + match url_command { + UrlCommand::Add(_) => { + // Add URL. + } + UrlCommand::Remove(_) => { + // Remove URL if it exists. + } + UrlCommand::Set(_) => { + // Set the URLs regardless of previous (if any) value. + } + UrlCommand::Clear(_) => { + // Remove the URLs. + } + } + } + ArtistCommand::Bandcamp(url_command) => { + match url_command { + UrlCommand::Add(_) => { + // Add URL. + } + UrlCommand::Remove(_) => { + // Remove URL if it exists. + } + UrlCommand::Set(_) => { + // Set the URLs regardless of previous (if any) value. + } + UrlCommand::Clear(_) => { + // Remove the URLs. + } + } + } + ArtistCommand::Qobuz(url_command) => { + match url_command { + UrlCommand::Add(_) => { + // Add URL or return error if one already existss. + } + UrlCommand::Remove(_) => { + // Remove URL if it exists. + } + UrlCommand::Set(_) => { + // Set the URL regardless of previous (if any) value. + } + UrlCommand::Clear(_) => { + // Remove the URL. + } + } + } + } + } + } + + music_hoard + .save_to_database() + .expect("failed to save database"); +} diff --git a/src/lib.rs b/src/lib.rs index 93fe3c6..ec1ae65 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -279,6 +279,18 @@ pub struct ArtistId { pub name: String, } +impl ArtistId { + pub fn new>(name: S) -> ArtistId { + ArtistId { name: name.into() } + } +} + +impl fmt::Display for ArtistId { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.name) + } +} + /// The artist properties. #[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)] pub struct ArtistProperties { @@ -307,6 +319,58 @@ pub struct Artist { pub albums: Vec, } +impl Artist { + pub fn new(id: ArtistId) -> Self { + Artist { + id, + properties: ArtistProperties::default(), + albums: vec![], + } + } + + fn add_musicbrainz_url>(&mut self, url: S) -> Result<(), Error> { + if self.properties.musicbrainz.is_some() { + return Err(Error::CollectionError(format!( + "artist '{}' already has a MusicBrainz URL", + self.id + ))); + } + + self.properties.musicbrainz = Some(MusicBrainz::new(url)?); + Ok(()) + } + + fn remove_musicbrainz_url>(&mut self, url: S) -> Result<(), Error> { + if self.properties.musicbrainz.is_none() { + return Err(Error::CollectionError(format!( + "artist '{}' does not have a MusicBrainz URL", + self.id + ))); + } + + if self.properties.musicbrainz.as_ref().unwrap().0.as_str() == url.as_ref() { + self.properties.musicbrainz = None; + Ok(()) + } else { + Err(Error::CollectionError(format!( + "artist '{}' does not have this MusicBrainz URL {}", + self.id, + url.as_ref(), + ))) + } + } + + fn set_musicbrainz_url>(&mut self, url: S) -> Result<(), Error> { + self.properties.musicbrainz = Some(MusicBrainz::new(url)?); + Ok(()) + } + + fn clear_musicbrainz_url(&mut self) -> Result<(), Error> { + self.properties.musicbrainz = None; + Ok(()) + } +} + impl PartialOrd for Artist { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) @@ -398,6 +462,8 @@ where /// Error type for `musichoard`. #[derive(Debug, PartialEq, Eq)] pub enum Error { + /// The [`MusicHoard`] is not able to read/write its in-memory collection. + CollectionError(String), /// The [`MusicHoard`] failed to read/write from/to the library. LibraryError(String), /// The [`MusicHoard`] failed to read/write from/to the database. @@ -411,6 +477,7 @@ pub enum Error { impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { + Self::CollectionError(ref s) => write!(f, "failed to read/write the collection: {s}"), Self::LibraryError(ref s) => write!(f, "failed to read/write from/to the library: {s}"), Self::DatabaseError(ref s) => { write!(f, "failed to read/write from/to the database: {s}") @@ -572,11 +639,7 @@ impl MusicHoard { .unwrap() } else { album_ids.insert(artist_id.clone(), HashSet::::new()); - artists.push(Artist { - id: artist_id.clone(), - properties: ArtistProperties::default(), - albums: vec![], - }); + artists.push(Artist::new(artist_id.clone())); artists.last_mut().unwrap() }; @@ -605,6 +668,96 @@ impl MusicHoard { artists } + + pub fn new_artist(&mut self, artist_id: ArtistId) -> Result<(), Error> { + // We want to return an error if the artist already exists so we first do a check. + let artists: &Vec = &self.collection; + + if let Some(ref a) = artists.iter().find(|a| a.id == artist_id) { + return Err(Error::CollectionError(format!( + "artist '{}' is already in the collection", + a.id + ))); + } + + let new_artist = vec![Artist::new(artist_id)]; + + let collection = mem::take(&mut self.collection); + self.collection = Self::merge(collection, new_artist); + + Ok(()) + } + + pub fn delete_artist(&mut self, artist_id: ArtistId) -> Result<(), Error> { + let index_opt = self.collection.iter().position(|a| a.id == artist_id); + + match index_opt { + Some(index) => { + self.collection.remove(index); + Ok(()) + } + None => Err(Error::CollectionError(format!( + "artist '{}' is not in the collection", + artist_id + ))), + } + } + + pub fn add_musicbrainz_url>( + &mut self, + artist_id: ArtistId, + url: S, + ) -> Result<(), Error> { + let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); + match artist_opt { + Some(ref mut artist) => artist.add_musicbrainz_url(url), + None => Err(Error::CollectionError(format!( + "artist '{}' is not in the collection", + artist_id + ))), + } + } + + pub fn remove_musicbrainz_url>( + &mut self, + artist_id: ArtistId, + url: S, + ) -> Result<(), Error> { + let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); + match artist_opt { + Some(ref mut artist) => artist.remove_musicbrainz_url(url), + None => Err(Error::CollectionError(format!( + "artist '{}' is not in the collection", + artist_id + ))), + } + } + + pub fn set_musicbrainz_url>( + &mut self, + artist_id: ArtistId, + url: S, + ) -> Result<(), Error> { + let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); + match artist_opt { + Some(ref mut artist) => artist.set_musicbrainz_url(url), + None => Err(Error::CollectionError(format!( + "artist '{}' is not in the collection", + artist_id + ))), + } + } + + pub fn clear_musicbrainz_url(&mut self, artist_id: ArtistId) -> Result<(), Error> { + let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); + match artist_opt { + Some(ref mut artist) => artist.clear_musicbrainz_url(), + None => Err(Error::CollectionError(format!( + "artist '{}' is not in the collection", + artist_id + ))), + } + } } #[cfg(test)] -- 2.45.2 From d8a1ec1645991b4d54fc9068561c7166895796f5 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 7 Jan 2024 16:27:28 +0100 Subject: [PATCH 02/16] Reduce code repetition --- src/lib.rs | 166 ++++++++++++++++++++++++----------------------------- 1 file changed, 76 insertions(+), 90 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index ec1ae65..b7cb440 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -542,6 +542,10 @@ impl MusicHoard { } } + pub fn get_collection(&self) -> &Collection { + &self.collection + } + pub fn rescan_library(&mut self) -> Result<(), Error> { match self.library { Some(ref mut library) => { @@ -584,8 +588,73 @@ impl MusicHoard { } } - pub fn get_collection(&self) -> &Collection { - &self.collection + pub fn new_artist>(&mut self, artist_id: ID) -> Result<(), Error> { + if let Ok(artist) = self.get_artist_or_err(artist_id.as_ref()) { + return Err(Error::CollectionError(format!( + "artist '{}' is already in the collection", + artist.id + ))); + } + + let new_artist = vec![Artist::new(artist_id.as_ref().clone())]; + + let collection = mem::take(&mut self.collection); + self.collection = Self::merge(collection, new_artist); + + Ok(()) + } + + pub fn delete_artist>(&mut self, artist_id: ID) -> Result<(), Error> { + let index_opt = self + .collection + .iter() + .position(|a| &a.id == artist_id.as_ref()); + + match index_opt { + Some(index) => { + self.collection.remove(index); + Ok(()) + } + None => Err(Error::CollectionError(format!( + "artist '{}' is not in the collection", + artist_id.as_ref() + ))), + } + } + + pub fn add_musicbrainz_url, S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .add_musicbrainz_url(url) + } + + pub fn remove_musicbrainz_url, S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .remove_musicbrainz_url(url) + } + + pub fn set_musicbrainz_url, S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .set_musicbrainz_url(url) + } + + pub fn clear_musicbrainz_url>( + &mut self, + artist_id: ID, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .clear_musicbrainz_url() } fn sort(collection: &mut [Artist]) { @@ -669,94 +738,11 @@ impl MusicHoard { artists } - pub fn new_artist(&mut self, artist_id: ArtistId) -> Result<(), Error> { - // We want to return an error if the artist already exists so we first do a check. - let artists: &Vec = &self.collection; - - if let Some(ref a) = artists.iter().find(|a| a.id == artist_id) { - return Err(Error::CollectionError(format!( - "artist '{}' is already in the collection", - a.id - ))); - } - - let new_artist = vec![Artist::new(artist_id)]; - - let collection = mem::take(&mut self.collection); - self.collection = Self::merge(collection, new_artist); - - Ok(()) - } - - pub fn delete_artist(&mut self, artist_id: ArtistId) -> Result<(), Error> { - let index_opt = self.collection.iter().position(|a| a.id == artist_id); - - match index_opt { - Some(index) => { - self.collection.remove(index); - Ok(()) - } - None => Err(Error::CollectionError(format!( - "artist '{}' is not in the collection", - artist_id - ))), - } - } - - pub fn add_musicbrainz_url>( - &mut self, - artist_id: ArtistId, - url: S, - ) -> Result<(), Error> { - let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); - match artist_opt { - Some(ref mut artist) => artist.add_musicbrainz_url(url), - None => Err(Error::CollectionError(format!( - "artist '{}' is not in the collection", - artist_id - ))), - } - } - - pub fn remove_musicbrainz_url>( - &mut self, - artist_id: ArtistId, - url: S, - ) -> Result<(), Error> { - let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); - match artist_opt { - Some(ref mut artist) => artist.remove_musicbrainz_url(url), - None => Err(Error::CollectionError(format!( - "artist '{}' is not in the collection", - artist_id - ))), - } - } - - pub fn set_musicbrainz_url>( - &mut self, - artist_id: ArtistId, - url: S, - ) -> Result<(), Error> { - let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); - match artist_opt { - Some(ref mut artist) => artist.set_musicbrainz_url(url), - None => Err(Error::CollectionError(format!( - "artist '{}' is not in the collection", - artist_id - ))), - } - } - - pub fn clear_musicbrainz_url(&mut self, artist_id: ArtistId) -> Result<(), Error> { - let mut artist_opt = self.collection.iter_mut().find(|a| a.id == artist_id); - match artist_opt { - Some(ref mut artist) => artist.clear_musicbrainz_url(), - None => Err(Error::CollectionError(format!( - "artist '{}' is not in the collection", - artist_id - ))), - } + fn get_artist_or_err(&mut self, artist_id: &ArtistId) -> Result<&mut Artist, Error> { + let artist_opt = self.collection.iter_mut().find(|a| &a.id == artist_id); + artist_opt.ok_or_else(|| { + Error::CollectionError(format!("artist '{}' is not in the collection", artist_id)) + }) } } -- 2.45.2 From bbab89d25f2d9bdc7e1444c8d90ff485c9cdc25b Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 7 Jan 2024 17:01:23 +0100 Subject: [PATCH 03/16] Prepare to reduce code repetition in mh-edit --- src/bin/mh-edit.rs | 210 +++++++++++++++++++++++---------------------- src/lib.rs | 6 ++ 2 files changed, 115 insertions(+), 101 deletions(-) diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index 58493c5..b79346c 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -1,16 +1,14 @@ -use std::fs::OpenOptions; use std::path::PathBuf; use structopt::StructOpt; use musichoard::{ - database::{ - json::{backend::JsonDatabaseFileBackend, JsonDatabase}, - IDatabase, - }, + database::json::{backend::JsonDatabaseFileBackend, JsonDatabase}, library::NoLibrary, - Artist, ArtistId, ArtistProperties, MusicHoard, + ArtistId, MusicHoard, }; +type MH = MusicHoard>; + #[derive(StructOpt, Debug)] struct Opt { #[structopt(subcommand)] @@ -29,6 +27,14 @@ enum Category { Artist(ArtistCommand), } +impl Category { + fn handle(self, music_hoard: &mut MH) { + match self { + Category::Artist(artist_command) => artist_command.handle(music_hoard), + } + } +} + #[derive(StructOpt, Debug)] enum ArtistCommand { New(ArtistValue), @@ -66,6 +72,102 @@ struct MultiUrlValue { urls: Vec, } +macro_rules! url_command_dispatch { + ($command:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident) => { + match $command { + UrlCommand::Add(single_url_value) => { + $mh.$add(ArtistId::new(single_url_value.artist), single_url_value.url) + .expect("failed to add URL(s)"); + } + UrlCommand::Remove(single_url_value) => { + $mh.$remove(ArtistId::new(single_url_value.artist), single_url_value.url) + .expect("failed to remove URL(s)"); + } + UrlCommand::Set(single_url_value) => { + $mh.$set(ArtistId::new(single_url_value.artist), single_url_value.url) + .expect("failed to set URL(s)"); + } + UrlCommand::Clear(artist_value) => { + $mh.$clear(ArtistId::new(artist_value.artist)) + .expect("failed to clear URL(s)"); + } + } + }; +} + +impl ArtistCommand { + fn handle(self, music_hoard: &mut MH) { + match self { + ArtistCommand::New(artist_value) => { + music_hoard + .new_artist(ArtistId::new(artist_value.artist)) + .expect("failed to add new artist"); + } + ArtistCommand::Delete(artist_value) => { + music_hoard + .delete_artist(ArtistId::new(artist_value.artist)) + .expect("failed to delete artist"); + } + ArtistCommand::MusicBrainz(url_command) => url_command_dispatch!( + url_command, + music_hoard, + add_musicbrainz_url, + remove_musicbrainz_url, + set_musicbrainz_url, + clear_musicbrainz_url + ), + ArtistCommand::MusicButler(url_command) => { + match url_command { + UrlCommand::Add(_) => { + // Add URL. + } + UrlCommand::Remove(_) => { + // Remove URL if it exists. + } + UrlCommand::Set(_) => { + // Set the URLs regardless of previous (if any) value. + } + UrlCommand::Clear(_) => { + // Remove the URLs. + } + } + } + ArtistCommand::Bandcamp(url_command) => { + match url_command { + UrlCommand::Add(_) => { + // Add URL. + } + UrlCommand::Remove(_) => { + // Remove URL if it exists. + } + UrlCommand::Set(_) => { + // Set the URLs regardless of previous (if any) value. + } + UrlCommand::Clear(_) => { + // Remove the URLs. + } + } + } + ArtistCommand::Qobuz(url_command) => { + match url_command { + UrlCommand::Add(_) => { + // Add URL or return error if one already existss. + } + UrlCommand::Remove(_) => { + // Remove URL if it exists. + } + UrlCommand::Set(_) => { + // Set the URL regardless of previous (if any) value. + } + UrlCommand::Clear(_) => { + // Remove the URL. + } + } + } + } + } +} + fn main() { let opt = Opt::from_args(); @@ -79,101 +181,7 @@ fn main() { .load_from_database() .expect("failed to load database"); - match opt.category { - Category::Artist(artist_command) => { - match artist_command { - ArtistCommand::New(artist_value) => { - music_hoard - .new_artist(ArtistId::new(artist_value.artist)) - .expect("failed to add new artist"); - } - ArtistCommand::Delete(artist_value) => { - music_hoard - .delete_artist(ArtistId::new(artist_value.artist)) - .expect("failed to delete artist"); - } - ArtistCommand::MusicBrainz(url_command) => match url_command { - UrlCommand::Add(single_url_value) => { - music_hoard - .add_musicbrainz_url( - ArtistId::new(single_url_value.artist), - single_url_value.url, - ) - .expect("failed to add MusicBrainz URL"); - } - UrlCommand::Remove(single_url_value) => { - music_hoard - .remove_musicbrainz_url( - ArtistId::new(single_url_value.artist), - single_url_value.url, - ) - .expect("failed to remove MusicBrainz URL"); - } - UrlCommand::Set(single_url_value) => { - music_hoard - .set_musicbrainz_url( - ArtistId::new(single_url_value.artist), - single_url_value.url, - ) - .expect("failed to set MusicBrainz URL"); - } - UrlCommand::Clear(artist_value) => { - music_hoard - .clear_musicbrainz_url(ArtistId::new(artist_value.artist)) - .expect("failed to clear MusicBrainz URL"); - } - }, - ArtistCommand::MusicButler(url_command) => { - match url_command { - UrlCommand::Add(_) => { - // Add URL. - } - UrlCommand::Remove(_) => { - // Remove URL if it exists. - } - UrlCommand::Set(_) => { - // Set the URLs regardless of previous (if any) value. - } - UrlCommand::Clear(_) => { - // Remove the URLs. - } - } - } - ArtistCommand::Bandcamp(url_command) => { - match url_command { - UrlCommand::Add(_) => { - // Add URL. - } - UrlCommand::Remove(_) => { - // Remove URL if it exists. - } - UrlCommand::Set(_) => { - // Set the URLs regardless of previous (if any) value. - } - UrlCommand::Clear(_) => { - // Remove the URLs. - } - } - } - ArtistCommand::Qobuz(url_command) => { - match url_command { - UrlCommand::Add(_) => { - // Add URL or return error if one already existss. - } - UrlCommand::Remove(_) => { - // Remove URL if it exists. - } - UrlCommand::Set(_) => { - // Set the URL regardless of previous (if any) value. - } - UrlCommand::Clear(_) => { - // Remove the URL. - } - } - } - } - } - } + opt.category.handle(&mut music_hoard); music_hoard .save_to_database() diff --git a/src/lib.rs b/src/lib.rs index b7cb440..97527a4 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -279,6 +279,12 @@ pub struct ArtistId { pub name: String, } +impl AsRef for ArtistId { + fn as_ref(&self) -> &ArtistId { + self + } +} + impl ArtistId { pub fn new>(name: S) -> ArtistId { ArtistId { name: name.into() } -- 2.45.2 From 3ea68d2935f59b3a479d786b852567d767df2959 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 8 Jan 2024 18:54:03 +0100 Subject: [PATCH 04/16] Add code for modifying musicbutler entries --- src/bin/mh-edit.rs | 44 +++++++++---------- src/lib.rs | 104 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 119 insertions(+), 29 deletions(-) diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index b79346c..5d0bb2e 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -73,18 +73,18 @@ struct MultiUrlValue { } macro_rules! url_command_dispatch { - ($command:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident) => { - match $command { - UrlCommand::Add(single_url_value) => { - $mh.$add(ArtistId::new(single_url_value.artist), single_url_value.url) + ($cmd:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident, $url:ident) => { + match $cmd { + UrlCommand::Add(url_value) => { + $mh.$add(ArtistId::new(url_value.artist), url_value.$url) .expect("failed to add URL(s)"); } - UrlCommand::Remove(single_url_value) => { - $mh.$remove(ArtistId::new(single_url_value.artist), single_url_value.url) + UrlCommand::Remove(url_value) => { + $mh.$remove(ArtistId::new(url_value.artist), url_value.$url) .expect("failed to remove URL(s)"); } - UrlCommand::Set(single_url_value) => { - $mh.$set(ArtistId::new(single_url_value.artist), single_url_value.url) + UrlCommand::Set(url_value) => { + $mh.$set(ArtistId::new(url_value.artist), url_value.$url) .expect("failed to set URL(s)"); } UrlCommand::Clear(artist_value) => { @@ -114,24 +114,18 @@ impl ArtistCommand { add_musicbrainz_url, remove_musicbrainz_url, set_musicbrainz_url, - clear_musicbrainz_url + clear_musicbrainz_url, + url + ), + ArtistCommand::MusicButler(url_command) => url_command_dispatch!( + url_command, + music_hoard, + add_musicbutler_urls, + remove_musicbutler_urls, + set_musicbutler_urls, + clear_musicbutler_urls, + urls ), - ArtistCommand::MusicButler(url_command) => { - match url_command { - UrlCommand::Add(_) => { - // Add URL. - } - UrlCommand::Remove(_) => { - // Remove URL if it exists. - } - UrlCommand::Set(_) => { - // Set the URLs regardless of previous (if any) value. - } - UrlCommand::Clear(_) => { - // Remove the URLs. - } - } - } ArtistCommand::Bandcamp(url_command) => { match url_command { UrlCommand::Add(_) => { diff --git a/src/lib.rs b/src/lib.rs index 97527a4..02f93be 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -375,6 +375,65 @@ impl Artist { self.properties.musicbrainz = None; Ok(()) } + + fn add_musicbutler_urls>(&mut self, urls: Vec) -> Result<(), Error> { + // Convert into URLs first to facilitate later comparison. + let urls: Result, Error> = urls.iter().map(MusicButler::new).collect(); + let mut urls = urls?; + + // Do not check and insert. First check if any of the provided URLs already exist so that + // the vector remains unchanged in case of failure. + let overlap: Vec<&MusicButler> = urls + .iter() + .filter(|url| self.properties.musicbutler.contains(url)) + .collect(); + if !overlap.is_empty() { + return Err(Error::CollectionError(format!( + "artist '{}' already has these MusicButler URL(s): {:?}", + self.id, overlap + ))); + } + + self.properties.musicbutler.append(&mut urls); + Ok(()) + } + + fn remove_musicbutler_urls>(&mut self, urls: Vec) -> Result<(), Error> { + // Convert into URLs first to facilitate later comparison. + let urls: Result, Error> = urls.iter().map(MusicButler::new).collect(); + let urls = urls?; + + // Do not check and insert. First check if any of the provided URLs already exist so that + // the vector remains unchanged in case of failure. + let difference: Vec<&MusicButler> = urls + .iter() + .filter(|url| !self.properties.musicbutler.contains(url)) + .collect(); + if !difference.is_empty() { + return Err(Error::CollectionError(format!( + "artist '{}' does not have these MusicButler URL(s): {:?}", + self.id, difference + ))); + } + + let musicbutler = mem::take(&mut self.properties.musicbutler); + self.properties.musicbutler = musicbutler + .into_iter() + .filter(|url| !urls.contains(url)) + .collect(); + Ok(()) + } + + fn set_musicbutler_urls>(&mut self, urls: Vec) -> Result<(), Error> { + let urls: Result, Error> = urls.iter().map(MusicButler::new).collect(); + self.properties.musicbutler = urls?; + Ok(()) + } + + fn clear_musicbutler_urls(&mut self) -> Result<(), Error> { + self.properties.musicbutler.clear(); + Ok(()) + } } impl PartialOrd for Artist { @@ -663,6 +722,41 @@ impl MusicHoard { .clear_musicbrainz_url() } + pub fn add_musicbutler_urls, S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .add_musicbutler_urls(urls) + } + + pub fn remove_musicbutler_urls, S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .remove_musicbutler_urls(urls) + } + + pub fn set_musicbutler_urls, S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .set_musicbutler_urls(urls) + } + + pub fn clear_musicbutler_urls>( + &mut self, + artist_id: ID, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())? + .clear_musicbutler_urls() + } + fn sort(collection: &mut [Artist]) { collection.sort_unstable(); for artist in collection.iter_mut() { @@ -745,10 +839,12 @@ impl MusicHoard { } fn get_artist_or_err(&mut self, artist_id: &ArtistId) -> Result<&mut Artist, Error> { - let artist_opt = self.collection.iter_mut().find(|a| &a.id == artist_id); - artist_opt.ok_or_else(|| { - Error::CollectionError(format!("artist '{}' is not in the collection", artist_id)) - }) + self.collection + .iter_mut() + .find(|a| &a.id == artist_id) + .ok_or_else(|| { + Error::CollectionError(format!("artist '{}' is not in the collection", artist_id)) + }) } } -- 2.45.2 From 326b876d9c8194a700f6d39f4f3f1819b4bb5f95 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 8 Jan 2024 22:14:25 +0100 Subject: [PATCH 05/16] Add code for remaining urls --- Cargo.lock | 12 +- Cargo.toml | 1 + src/bin/mh-edit.rs | 50 ++---- src/lib.rs | 434 +++++++++++++++++++++++++++++++++------------ 4 files changed, 349 insertions(+), 148 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 491d365..890b6ad 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -273,6 +273,15 @@ dependencies = [ "either", ] +[[package]] +name = "itertools" +version = "0.12.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "25db6b064527c5d482d0423354fcd07a89a2dfe07b67892e62411946db7f07b0" +dependencies = [ + "either", +] + [[package]] name = "itoa" version = "1.0.6" @@ -366,6 +375,7 @@ name = "musichoard" version = "0.1.0" dependencies = [ "crossterm", + "itertools 0.12.0", "mockall", "once_cell", "openssh", @@ -476,7 +486,7 @@ checksum = "59230a63c37f3e18569bdb90e4a89cbf5bf8b06fea0b84e65ea10cc4df47addd" dependencies = [ "difflib", "float-cmp", - "itertools", + "itertools 0.10.5", "normalize-line-endings", "predicates-core", "regex", diff --git a/Cargo.toml b/Cargo.toml index 395c326..e9d6add 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] crossterm = { version = "0.26.1", optional = true} +itertools = { version = "0.12.0" } openssh = { version = "0.9.9", features = ["native-mux"], default-features = false, optional = true} ratatui = { version = "0.20.1", optional = true} serde = { version = "1.0.159", features = ["derive"] } diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index 5d0bb2e..60d9bcc 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -126,38 +126,24 @@ impl ArtistCommand { clear_musicbutler_urls, urls ), - ArtistCommand::Bandcamp(url_command) => { - match url_command { - UrlCommand::Add(_) => { - // Add URL. - } - UrlCommand::Remove(_) => { - // Remove URL if it exists. - } - UrlCommand::Set(_) => { - // Set the URLs regardless of previous (if any) value. - } - UrlCommand::Clear(_) => { - // Remove the URLs. - } - } - } - ArtistCommand::Qobuz(url_command) => { - match url_command { - UrlCommand::Add(_) => { - // Add URL or return error if one already existss. - } - UrlCommand::Remove(_) => { - // Remove URL if it exists. - } - UrlCommand::Set(_) => { - // Set the URL regardless of previous (if any) value. - } - UrlCommand::Clear(_) => { - // Remove the URL. - } - } - } + ArtistCommand::Bandcamp(url_command) => url_command_dispatch!( + url_command, + music_hoard, + add_bandcamp_urls, + remove_bandcamp_urls, + set_bandcamp_urls, + clear_bandcamp_urls, + urls + ), + ArtistCommand::Qobuz(url_command) => url_command_dispatch!( + url_command, + music_hoard, + add_qobuz_url, + remove_qobuz_url, + set_qobuz_url, + clear_qobuz_url, + url + ), } } } diff --git a/src/lib.rs b/src/lib.rs index 02f93be..52b86a7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,12 +6,13 @@ pub mod library; use std::{ cmp::Ordering, collections::{HashMap, HashSet}, - fmt, + fmt::{self, Debug}, iter::Peekable, mem, }; use database::IDatabase; +use itertools::Itertools; use library::{ILibrary, Item, Query}; use serde::{Deserialize, Serialize}; use url::Url; @@ -79,6 +80,20 @@ impl MusicBrainz { } } +impl TryFrom<&str> for MusicBrainz { + type Error = Error; + + fn try_from(value: &str) -> Result { + MusicBrainz::new(value) + } +} + +impl AsRef for MusicBrainz { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + impl IUrl for MusicBrainz { fn url(&self) -> &str { self.0.as_str() @@ -119,6 +134,20 @@ impl MusicButler { } } +impl TryFrom<&str> for MusicButler { + type Error = Error; + + fn try_from(value: &str) -> Result { + MusicButler::new(value) + } +} + +impl AsRef for MusicButler { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + impl IUrl for MusicButler { fn url(&self) -> &str { self.0.as_str() @@ -152,6 +181,20 @@ impl Bandcamp { } } +impl TryFrom<&str> for Bandcamp { + type Error = Error; + + fn try_from(value: &str) -> Result { + Bandcamp::new(value) + } +} + +impl AsRef for Bandcamp { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + impl IUrl for Bandcamp { fn url(&self) -> &str { self.0.as_str() @@ -185,6 +228,20 @@ impl Qobuz { } } +impl TryFrom<&str> for Qobuz { + type Error = Error; + + fn try_from(value: &str) -> Result { + Qobuz::new(value) + } +} + +impl AsRef for Qobuz { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} + impl IUrl for Qobuz { fn url(&self) -> &str { self.0.as_str() @@ -325,6 +382,46 @@ pub struct Artist { pub albums: Vec, } +macro_rules! artist_unique_url_dispatch { + ($add:ident, $remove:ident, $set:ident, $clear:ident, $label:literal, $field:ident) => { + fn $add>(&mut self, url: S) -> Result<(), Error> { + Self::add_unique_url(&self.id, $label, &mut self.properties.$field, url) + } + + fn $remove>(&mut self, url: S) -> Result<(), Error> { + Self::remove_unique_url(&self.id, $label, &mut self.properties.$field, url) + } + + fn $set>(&mut self, url: S) -> Result<(), Error> { + Self::set_unique_url(&mut self.properties.$field, url) + } + + fn $clear(&mut self) { + Self::clear_unique_url(&mut self.properties.$field); + } + }; +} + +macro_rules! artist_multi_url_dispatch { + ($add:ident, $remove:ident, $set:ident, $clear:ident, $label:literal, $field:ident) => { + fn $add>(&mut self, urls: Vec) -> Result<(), Error> { + Self::add_multi_urls(&self.id, $label, &mut self.properties.$field, urls) + } + + fn $remove>(&mut self, urls: Vec) -> Result<(), Error> { + Self::remove_multi_urls(&self.id, $label, &mut self.properties.$field, urls) + } + + fn $set>(&mut self, urls: Vec) -> Result<(), Error> { + Self::set_multi_urls(&mut self.properties.$field, urls) + } + + fn $clear(&mut self) { + Self::clear_multi_urls(&mut self.properties.$field); + } + }; +} + impl Artist { pub fn new(id: ArtistId) -> Self { Artist { @@ -334,106 +431,189 @@ impl Artist { } } - fn add_musicbrainz_url>(&mut self, url: S) -> Result<(), Error> { - if self.properties.musicbrainz.is_some() { - return Err(Error::CollectionError(format!( - "artist '{}' already has a MusicBrainz URL", - self.id - ))); + fn add_unique_url< + S: AsRef, + T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, + >( + artist_id: &ArtistId, + label: &'static str, + container: &mut Option, + url: S, + ) -> Result<(), Error> { + let url: T = url.as_ref().try_into()?; + + if let Some(current) = container { + if current == &url { + return Err(Error::CollectionError(format!( + "artist '{}' already has this {} URL: {}", + artist_id, + label, + current.as_ref() + ))); + } else { + return Err(Error::CollectionError(format!( + "artist '{}' already has a different {} URL: {}", + artist_id, + label, + current.as_ref() + ))); + } } - self.properties.musicbrainz = Some(MusicBrainz::new(url)?); + _ = container.insert(url); Ok(()) } - fn remove_musicbrainz_url>(&mut self, url: S) -> Result<(), Error> { - if self.properties.musicbrainz.is_none() { + fn remove_unique_url< + S: AsRef, + T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, + >( + artist_id: &ArtistId, + label: &'static str, + container: &mut Option, + url: S, + ) -> Result<(), Error> { + if container.is_none() { return Err(Error::CollectionError(format!( - "artist '{}' does not have a MusicBrainz URL", - self.id + "artist '{}' does not have a {} URL", + artist_id, label ))); } - if self.properties.musicbrainz.as_ref().unwrap().0.as_str() == url.as_ref() { - self.properties.musicbrainz = None; + let url: T = url.as_ref().try_into()?; + if container.as_ref().unwrap() == &url { + _ = container.take(); Ok(()) } else { Err(Error::CollectionError(format!( - "artist '{}' does not have this MusicBrainz URL {}", - self.id, + "artist '{}' does not have this {} URL: {}", + artist_id, + label, url.as_ref(), ))) } } - fn set_musicbrainz_url>(&mut self, url: S) -> Result<(), Error> { - self.properties.musicbrainz = Some(MusicBrainz::new(url)?); + fn set_unique_url, T: for<'a> TryFrom<&'a str, Error = Error>>( + container: &mut Option, + url: S, + ) -> Result<(), Error> { + _ = container.insert(url.as_ref().try_into()?); Ok(()) } - fn clear_musicbrainz_url(&mut self) -> Result<(), Error> { - self.properties.musicbrainz = None; - Ok(()) + fn clear_unique_url(container: &mut Option) { + _ = container.take(); } - fn add_musicbutler_urls>(&mut self, urls: Vec) -> Result<(), Error> { + fn add_multi_urls< + S: AsRef, + T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, + >( + artist_id: &ArtistId, + label: &'static str, + container: &mut Vec, + urls: Vec, + ) -> Result<(), Error> { // Convert into URLs first to facilitate later comparison. - let urls: Result, Error> = urls.iter().map(MusicButler::new).collect(); + let urls: Result, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect(); let mut urls = urls?; // Do not check and insert. First check if any of the provided URLs already exist so that // the vector remains unchanged in case of failure. - let overlap: Vec<&MusicButler> = urls - .iter() - .filter(|url| self.properties.musicbutler.contains(url)) - .collect(); + let overlap: Vec<&T> = urls.iter().filter(|url| container.contains(url)).collect(); if !overlap.is_empty() { return Err(Error::CollectionError(format!( - "artist '{}' already has these MusicButler URL(s): {:?}", - self.id, overlap + "artist '{}' already has these {} URL(s): {}", + artist_id, + label, + overlap.iter().map(|url| url.as_ref()).format(", ") ))); } - self.properties.musicbutler.append(&mut urls); + container.append(&mut urls); Ok(()) } - fn remove_musicbutler_urls>(&mut self, urls: Vec) -> Result<(), Error> { + fn remove_multi_urls< + S: AsRef, + T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, + >( + artist_id: &ArtistId, + label: &'static str, + container: &mut Vec, + urls: Vec, + ) -> Result<(), Error> { // Convert into URLs first to facilitate later comparison. - let urls: Result, Error> = urls.iter().map(MusicButler::new).collect(); + let urls: Result, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect(); let urls = urls?; // Do not check and insert. First check if any of the provided URLs already exist so that // the vector remains unchanged in case of failure. - let difference: Vec<&MusicButler> = urls - .iter() - .filter(|url| !self.properties.musicbutler.contains(url)) - .collect(); + let difference: Vec<&T> = urls.iter().filter(|url| !container.contains(url)).collect(); if !difference.is_empty() { return Err(Error::CollectionError(format!( - "artist '{}' does not have these MusicButler URL(s): {:?}", - self.id, difference + "artist '{}' does not have these {} URL(s): {}", + artist_id, + label, + difference.iter().map(|url| url.as_ref()).format(", ") ))); } - let musicbutler = mem::take(&mut self.properties.musicbutler); - self.properties.musicbutler = musicbutler - .into_iter() - .filter(|url| !urls.contains(url)) - .collect(); + container.retain(|url| !urls.contains(url)); Ok(()) } - fn set_musicbutler_urls>(&mut self, urls: Vec) -> Result<(), Error> { - let urls: Result, Error> = urls.iter().map(MusicButler::new).collect(); - self.properties.musicbutler = urls?; + fn set_multi_urls, T: for<'a> TryFrom<&'a str, Error = Error>>( + container: &mut Vec, + urls: Vec, + ) -> Result<(), Error> { + let urls: Result, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect(); + let mut urls = urls?; + container.clear(); + container.append(&mut urls); Ok(()) } - fn clear_musicbutler_urls(&mut self) -> Result<(), Error> { - self.properties.musicbutler.clear(); - Ok(()) + fn clear_multi_urls(container: &mut Vec) { + container.clear(); } + + artist_unique_url_dispatch!( + add_musicbrainz_url, + remove_musicbrainz_url, + set_musicbrainz_url, + clear_musicbrainz_url, + "MusicBrainz", + musicbrainz + ); + + artist_multi_url_dispatch!( + add_musicbutler_urls, + remove_musicbutler_urls, + set_musicbutler_urls, + clear_musicbutler_urls, + "MusicButler", + musicbutler + ); + + artist_multi_url_dispatch!( + add_bandcamp_urls, + remove_bandcamp_urls, + set_bandcamp_urls, + clear_bandcamp_urls, + "Bandcamp", + bandcamp + ); + + artist_unique_url_dispatch!( + add_qobuz_url, + remove_qobuz_url, + set_qobuz_url, + clear_qobuz_url, + "Qobuz", + qobuz + ); } impl PartialOrd for Artist { @@ -597,6 +777,72 @@ pub struct MusicHoard { collection: Collection, } +macro_rules! music_hoard_unique_url_dispatch { + ($add:ident, $remove:ident, $set:ident, $clear:ident) => { + pub fn $add, S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$add(url) + } + + pub fn $remove, S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$remove(url) + } + + pub fn $set, S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$set(url) + } + + pub fn $clear>(&mut self, artist_id: ID) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$clear(); + Ok(()) + } + }; +} + +macro_rules! music_hoard_multi_url_dispatch { + ($add:ident, $remove:ident, $set:ident, $clear:ident) => { + pub fn $add, S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$add(urls) + } + + pub fn $remove, S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$remove(urls) + } + + pub fn $set, S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$set(urls) + } + + pub fn $clear>(&mut self, artist_id: ID) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.$clear(); + Ok(()) + } + }; +} + impl MusicHoard { /// Create a new [`MusicHoard`] with the provided [`ILibrary`] and [`IDatabase`]. pub fn new(library: Option, database: Option) -> Self { @@ -687,75 +933,33 @@ impl MusicHoard { } } - pub fn add_musicbrainz_url, S: AsRef>( - &mut self, - artist_id: ID, - url: S, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .add_musicbrainz_url(url) - } + music_hoard_unique_url_dispatch!( + add_musicbrainz_url, + remove_musicbrainz_url, + set_musicbrainz_url, + clear_musicbrainz_url + ); - pub fn remove_musicbrainz_url, S: AsRef>( - &mut self, - artist_id: ID, - url: S, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .remove_musicbrainz_url(url) - } + music_hoard_multi_url_dispatch!( + add_musicbutler_urls, + remove_musicbutler_urls, + set_musicbutler_urls, + clear_musicbutler_urls + ); - pub fn set_musicbrainz_url, S: AsRef>( - &mut self, - artist_id: ID, - url: S, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .set_musicbrainz_url(url) - } + music_hoard_multi_url_dispatch!( + add_bandcamp_urls, + remove_bandcamp_urls, + set_bandcamp_urls, + clear_bandcamp_urls + ); - pub fn clear_musicbrainz_url>( - &mut self, - artist_id: ID, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .clear_musicbrainz_url() - } - - pub fn add_musicbutler_urls, S: AsRef>( - &mut self, - artist_id: ID, - urls: Vec, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .add_musicbutler_urls(urls) - } - - pub fn remove_musicbutler_urls, S: AsRef>( - &mut self, - artist_id: ID, - urls: Vec, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .remove_musicbutler_urls(urls) - } - - pub fn set_musicbutler_urls, S: AsRef>( - &mut self, - artist_id: ID, - urls: Vec, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .set_musicbutler_urls(urls) - } - - pub fn clear_musicbutler_urls>( - &mut self, - artist_id: ID, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())? - .clear_musicbutler_urls() - } + music_hoard_unique_url_dispatch!( + add_qobuz_url, + remove_qobuz_url, + set_qobuz_url, + clear_qobuz_url + ); fn sort(collection: &mut [Artist]) { collection.sort_unstable(); -- 2.45.2 From d43f82d5ef4a71595512bd8c6f77aed8f202dccb Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 8 Jan 2024 22:47:38 +0100 Subject: [PATCH 06/16] Ignore the new binary source file --- .gitea/workflows/gitea-ci.yaml | 1 + README.md | 1 + 2 files changed, 2 insertions(+) diff --git a/.gitea/workflows/gitea-ci.yaml b/.gitea/workflows/gitea-ci.yaml index 72c9002..c701061 100644 --- a/.gitea/workflows/gitea-ci.yaml +++ b/.gitea/workflows/gitea-ci.yaml @@ -30,6 +30,7 @@ jobs: --ignore-not-existing --ignore "tests/*" --ignore "src/main.rs" + --ignore "src/bin/mh-edit.rs" --excl-start "GRCOV_EXCL_START|mod tests \{" --excl-stop "GRCOV_EXCL_STOP" --output-path ./target/debug/coverage/ diff --git a/README.md b/README.md index d9674e0..36f6a45 100644 --- a/README.md +++ b/README.md @@ -32,6 +32,7 @@ grcov codecov/debug/profraw \ --ignore-not-existing \ --ignore "tests/*" \ --ignore "src/main.rs" \ + --ignore "src/bin/mh-edit.rs" \ --excl-start "GRCOV_EXCL_START|mod tests \{" \ --excl-stop "GRCOV_EXCL_STOP" \ --output-path ./codecov/debug/coverage/ -- 2.45.2 From ac7a86f9251d3386a98df303035d635e416cb277 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Mon, 8 Jan 2024 22:52:50 +0100 Subject: [PATCH 07/16] Some code ergonomics --- src/bin/mh-edit.rs | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index 60d9bcc..205fb13 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -95,6 +95,18 @@ macro_rules! url_command_dispatch { }; } +macro_rules! single_url_command_dispatch { + ($cmd:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident) => { + url_command_dispatch!($cmd, $mh, $add, $remove, $set, $clear, url) + }; +} + +macro_rules! multi_url_command_dispatch { + ($cmd:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident) => { + url_command_dispatch!($cmd, $mh, $add, $remove, $set, $clear, urls) + }; +} + impl ArtistCommand { fn handle(self, music_hoard: &mut MH) { match self { @@ -108,41 +120,37 @@ impl ArtistCommand { .delete_artist(ArtistId::new(artist_value.artist)) .expect("failed to delete artist"); } - ArtistCommand::MusicBrainz(url_command) => url_command_dispatch!( + ArtistCommand::MusicBrainz(url_command) => single_url_command_dispatch!( url_command, music_hoard, add_musicbrainz_url, remove_musicbrainz_url, set_musicbrainz_url, - clear_musicbrainz_url, - url + clear_musicbrainz_url ), - ArtistCommand::MusicButler(url_command) => url_command_dispatch!( + ArtistCommand::MusicButler(url_command) => multi_url_command_dispatch!( url_command, music_hoard, add_musicbutler_urls, remove_musicbutler_urls, set_musicbutler_urls, - clear_musicbutler_urls, - urls + clear_musicbutler_urls ), - ArtistCommand::Bandcamp(url_command) => url_command_dispatch!( + ArtistCommand::Bandcamp(url_command) => multi_url_command_dispatch!( url_command, music_hoard, add_bandcamp_urls, remove_bandcamp_urls, set_bandcamp_urls, - clear_bandcamp_urls, - urls + clear_bandcamp_urls ), - ArtistCommand::Qobuz(url_command) => url_command_dispatch!( + ArtistCommand::Qobuz(url_command) => single_url_command_dispatch!( url_command, music_hoard, add_qobuz_url, remove_qobuz_url, set_qobuz_url, - clear_qobuz_url, - url + clear_qobuz_url ), } } -- 2.45.2 From 29ff4f9a00f9b9f499680ce9f63ac9f5f1346fc6 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Tue, 9 Jan 2024 23:45:21 +0100 Subject: [PATCH 08/16] Add unit tests --- src/lib.rs | 598 +++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 576 insertions(+), 22 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 52b86a7..b00757b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -423,9 +423,9 @@ macro_rules! artist_multi_url_dispatch { } impl Artist { - pub fn new(id: ArtistId) -> Self { + pub fn new>(id: ID) -> Self { Artist { - id, + id: id.into(), properties: ArtistProperties::default(), albums: vec![], } @@ -1068,6 +1068,18 @@ mod tests { use super::*; + static MUSICBRAINZ: &'static str = + "https://musicbrainz.org/artist/d368baa8-21ca-4759-9731-0b2753071ad8"; + static MUSICBRAINZ_2: &'static str = + "https://musicbrainz.org/artist/823869a5-5ded-4f6b-9fb7-2a9344d83c6b"; + static MUSICBUTLER: &'static str = "https://www.musicbutler.io/artist-page/483340948"; + static MUSICBUTLER_2: &'static str = "https://www.musicbutler.io/artist-page/658903042/"; + static BANDCAMP: &'static str = "https://thelasthangmen.bandcamp.com/"; + static BANDCAMP_2: &'static str = "https://viciouscrusade.bandcamp.com/"; + static QOBUZ: &'static str = "https://www.qobuz.com/nl-nl/interpreter/the-last-hangmen/1244413"; + static QOBUZ_2: &'static str = + "https://www.qobuz.com/nl-nl/interpreter/vicious-crusade/7522386"; + pub static COLLECTION: Lazy> = Lazy::new(|| collection!()); pub fn artist_to_items(artist: &Artist) -> Vec { @@ -1139,30 +1151,572 @@ mod tests { #[test] fn urls() { - let musicbrainz = "https://musicbrainz.org/artist/d368baa8-21ca-4759-9731-0b2753071ad8"; - let musicbutler = "https://www.musicbutler.io/artist-page/483340948"; - let bandcamp = "https://thelasthangmen.bandcamp.com/"; - let qobuz = "https://www.qobuz.com/nl-nl/interpreter/the-last-hangmen/1244413"; + assert!(MusicBrainz::new(MUSICBRAINZ).is_ok()); + assert!(MusicBrainz::new(MUSICBUTLER).is_err()); + assert!(MusicBrainz::new(BANDCAMP).is_err()); + assert!(MusicBrainz::new(QOBUZ).is_err()); - assert!(MusicBrainz::new(musicbrainz).is_ok()); - assert!(MusicBrainz::new(musicbutler).is_err()); - assert!(MusicBrainz::new(bandcamp).is_err()); - assert!(MusicBrainz::new(qobuz).is_err()); + assert!(MusicButler::new(MUSICBRAINZ).is_err()); + assert!(MusicButler::new(MUSICBUTLER).is_ok()); + assert!(MusicButler::new(BANDCAMP).is_err()); + assert!(MusicButler::new(QOBUZ).is_err()); - assert!(MusicButler::new(musicbrainz).is_err()); - assert!(MusicButler::new(musicbutler).is_ok()); - assert!(MusicButler::new(bandcamp).is_err()); - assert!(MusicButler::new(qobuz).is_err()); + assert!(Bandcamp::new(MUSICBRAINZ).is_err()); + assert!(Bandcamp::new(MUSICBUTLER).is_err()); + assert!(Bandcamp::new(BANDCAMP).is_ok()); + assert!(Bandcamp::new(QOBUZ).is_err()); - assert!(Bandcamp::new(musicbrainz).is_err()); - assert!(Bandcamp::new(musicbutler).is_err()); - assert!(Bandcamp::new(bandcamp).is_ok()); - assert!(Bandcamp::new(qobuz).is_err()); + assert!(Qobuz::new(MUSICBRAINZ).is_err()); + assert!(Qobuz::new(MUSICBUTLER).is_err()); + assert!(Qobuz::new(BANDCAMP).is_err()); + assert!(Qobuz::new(QOBUZ).is_ok()); + } - assert!(Qobuz::new(musicbrainz).is_err()); - assert!(Qobuz::new(musicbutler).is_err()); - assert!(Qobuz::new(bandcamp).is_err()); - assert!(Qobuz::new(qobuz).is_ok()); + #[test] + fn artist_new_delete() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let actual_err = music_hoard.new_artist(&artist_id).unwrap_err(); + let expected_err = Error::CollectionError(String::from( + "artist 'an artist' is already in the collection", + )); + assert_eq!(actual_err, expected_err); + assert_eq!(actual_err.to_string(), expected_err.to_string()); + + let actual_err = music_hoard.delete_artist(&artist_id_2).unwrap_err(); + let expected_err = Error::CollectionError(String::from( + "artist 'another artist' is not in the collection", + )); + assert_eq!(actual_err, expected_err); + assert_eq!(actual_err.to_string(), expected_err.to_string()); + + assert!(music_hoard.delete_artist(&artist_id).is_ok()); + } + + #[test] + fn artist_musicbrainz_url() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Option = None; + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Adding incorect URL is an error. + assert!(music_hoard + .add_musicbrainz_url(&artist_id, MUSICBUTLER) + .is_err()); + assert!(music_hoard + .add_musicbrainz_url(&artist_id, BANDCAMP) + .is_err()); + assert!(music_hoard.add_musicbrainz_url(&artist_id, QOBUZ).is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Adding URL to an artist not in the collection is an error. + assert!(music_hoard + .add_musicbrainz_url(&artist_id_2, MUSICBRAINZ) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Adding URL to artist. + assert!(music_hoard + .add_musicbrainz_url(&artist_id, MUSICBRAINZ) + .is_ok()); + _ = expected.insert(MusicBrainz::new(MUSICBRAINZ).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Adding further URLs is an error. + assert!(music_hoard + .add_musicbrainz_url(&artist_id, MUSICBRAINZ) + .is_err()); + assert!(music_hoard + .add_musicbrainz_url(&artist_id, MUSICBRAINZ_2) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Removing a URL from an artist not in the collection is an error. + assert!(music_hoard + .remove_musicbrainz_url(&artist_id_2, MUSICBRAINZ) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Removing a URL is only okay if it matches the stored one. + assert!(music_hoard + .remove_musicbrainz_url(&artist_id, MUSICBRAINZ_2) + .is_err()); + assert!(music_hoard + .remove_musicbrainz_url(&artist_id, MUSICBRAINZ) + .is_ok()); + _ = expected.take(); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Removing a URl if one does not exist is an error. + assert!(music_hoard + .remove_musicbrainz_url(&artist_id, MUSICBRAINZ) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Setting an incorrect URL is an error. + assert!(music_hoard + .set_musicbrainz_url(&artist_id, MUSICBUTLER) + .is_err()); + assert!(music_hoard + .set_musicbrainz_url(&artist_id, BANDCAMP) + .is_err()); + assert!(music_hoard.set_musicbrainz_url(&artist_id, QOBUZ).is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Setting a URL on an artist not in the collection is an error. + assert!(music_hoard + .set_musicbrainz_url(&artist_id_2, MUSICBRAINZ) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Setting a URL on an artist. + assert!(music_hoard + .set_musicbrainz_url(&artist_id, MUSICBRAINZ) + .is_ok()); + _ = expected.insert(MusicBrainz::new(MUSICBRAINZ).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + assert!(music_hoard + .set_musicbrainz_url(&artist_id, MUSICBRAINZ) + .is_ok()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + assert!(music_hoard + .set_musicbrainz_url(&artist_id, MUSICBRAINZ_2) + .is_ok()); + _ = expected.insert(MusicBrainz::new(MUSICBRAINZ_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Clearing URLs on an artist that does not exist is an error. + assert!(music_hoard.clear_musicbrainz_url(&artist_id_2).is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Clearing URLs. + assert!(music_hoard.clear_musicbrainz_url(&artist_id).is_ok()); + _ = expected.take(); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + assert!(music_hoard.delete_artist(&artist_id).is_ok()); + } + + #[test] + fn artist_musicbutler_urls() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Vec = vec![]; + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // If any URL is incorrect adding URLs is an error. + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBRAINZ, MUSICBRAINZ_2]) + .is_err()); + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_err()); + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![QOBUZ, QOBUZ_2]) + .is_err()); + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBRAINZ, MUSICBUTLER, BANDCAMP, QOBUZ]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Adding URLs to an artist not in the collection is an error. + assert!(music_hoard + .add_musicbutler_urls(&artist_id_2, vec![MUSICBUTLER]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Adding a URL. + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) + .is_ok()); + expected.push(MusicButler::new(MUSICBUTLER).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Adding a URL that already exists is an error. + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Adding another URL. + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) + .is_ok()); + expected.push(MusicButler::new(MUSICBUTLER_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Removing URLs to an artist not in the collection is an error. + assert!(music_hoard + .remove_musicbutler_urls(&artist_id_2, vec![MUSICBUTLER]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Removing a URL. + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) + .is_ok()); + expected.retain(|url| url.as_ref() != MUSICBUTLER); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // If any URL does not exist removing URLs is an error. + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // If any URL already exists exists adding URLs is an error. + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Removing a URL. + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) + .is_ok()); + expected.retain(|url| url.as_ref() != MUSICBUTLER_2); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Adding mutliple URLs is okay if none of them already exist. + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_ok()); + expected.push(MusicButler::new(MUSICBUTLER).unwrap()); + expected.push(MusicButler::new(MUSICBUTLER_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Removing multiple URLs is okay if they all exist. + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_ok()); + expected.clear(); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // If any URL is incorrect setting URLs is an error. + assert!(music_hoard + .set_musicbutler_urls(&artist_id, vec![MUSICBRAINZ, MUSICBRAINZ_2]) + .is_err()); + assert!(music_hoard + .set_musicbutler_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_err()); + assert!(music_hoard + .set_musicbutler_urls(&artist_id, vec![QOBUZ, QOBUZ_2]) + .is_err()); + assert!(music_hoard + .set_musicbutler_urls(&artist_id, vec![MUSICBRAINZ, MUSICBUTLER, BANDCAMP, QOBUZ]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Seting URL on an artist not in the collection is an error. + assert!(music_hoard + .set_musicbutler_urls(&artist_id_2, vec![MUSICBUTLER]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Set URLs. + assert!(music_hoard + .set_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) + .is_ok()); + expected.push(MusicButler::new(MUSICBUTLER).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + assert!(music_hoard + .set_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) + .is_ok()); + expected.clear(); + expected.push(MusicButler::new(MUSICBUTLER_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + assert!(music_hoard + .set_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_ok()); + expected.clear(); + expected.push(MusicButler::new(MUSICBUTLER).unwrap()); + expected.push(MusicButler::new(MUSICBUTLER_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Clearing URLs on an artist that does not exist is an error. + assert!(music_hoard.clear_musicbutler_urls(&artist_id_2).is_err()); + + // Clear URLs. + assert!(music_hoard.clear_musicbutler_urls(&artist_id).is_ok()); + expected.clear(); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + } + + #[test] + fn artist_bandcamp_urls() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Vec = vec![]; + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // If any URL is incorrect adding URLs is an error. + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![MUSICBRAINZ, MUSICBRAINZ_2]) + .is_err()); + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_err()); + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![QOBUZ, QOBUZ_2]) + .is_err()); + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![MUSICBRAINZ, MUSICBUTLER, BANDCAMP, QOBUZ]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Adding URLs to an artist not in the collection is an error. + assert!(music_hoard + .add_bandcamp_urls(&artist_id_2, vec![BANDCAMP]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Adding a URL. + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP]) + .is_ok()); + expected.push(Bandcamp::new(BANDCAMP).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Adding a URL that already exists is an error. + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Adding another URL. + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) + .is_ok()); + expected.push(Bandcamp::new(BANDCAMP_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Removing URLs to an artist not in the collection is an error. + assert!(music_hoard + .remove_bandcamp_urls(&artist_id_2, vec![BANDCAMP]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Removing a URL. + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP]) + .is_ok()); + expected.retain(|url| url.as_ref() != BANDCAMP); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // If any URL does not exist removing URLs is an error. + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // If any URL already exists exists adding URLs is an error. + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Removing a URL. + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) + .is_ok()); + expected.retain(|url| url.as_ref() != BANDCAMP_2); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Adding mutliple URLs is okay if none of them already exist. + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_ok()); + expected.push(Bandcamp::new(BANDCAMP).unwrap()); + expected.push(Bandcamp::new(BANDCAMP_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Removing multiple URLs is okay if they all exist. + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_ok()); + expected.clear(); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // If any URL is incorrect setting URLs is an error. + assert!(music_hoard + .set_bandcamp_urls(&artist_id, vec![MUSICBRAINZ, MUSICBRAINZ_2]) + .is_err()); + assert!(music_hoard + .set_bandcamp_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_err()); + assert!(music_hoard + .set_bandcamp_urls(&artist_id, vec![QOBUZ, QOBUZ_2]) + .is_err()); + assert!(music_hoard + .set_bandcamp_urls(&artist_id, vec![MUSICBRAINZ, MUSICBUTLER, BANDCAMP, QOBUZ]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Seting URL on an artist not in the collection is an error. + assert!(music_hoard + .set_bandcamp_urls(&artist_id_2, vec![BANDCAMP]) + .is_err()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Set URLs. + assert!(music_hoard + .set_bandcamp_urls(&artist_id, vec![BANDCAMP]) + .is_ok()); + expected.push(Bandcamp::new(BANDCAMP).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + assert!(music_hoard + .set_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) + .is_ok()); + expected.clear(); + expected.push(Bandcamp::new(BANDCAMP_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + assert!(music_hoard + .set_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_ok()); + expected.clear(); + expected.push(Bandcamp::new(BANDCAMP).unwrap()); + expected.push(Bandcamp::new(BANDCAMP_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Clearing URLs on an artist that does not exist is an error. + assert!(music_hoard.clear_bandcamp_urls(&artist_id_2).is_err()); + + // Clear URLs. + assert!(music_hoard.clear_bandcamp_urls(&artist_id).is_ok()); + expected.clear(); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + } + + #[test] + fn artist_qobuz_url() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Option = None; + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Adding incorect URL is an error. + assert!(music_hoard.add_qobuz_url(&artist_id, MUSICBRAINZ).is_err()); + assert!(music_hoard.add_qobuz_url(&artist_id, MUSICBUTLER).is_err()); + assert!(music_hoard.add_qobuz_url(&artist_id, BANDCAMP).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Adding URL to an artist not in the collection is an error. + assert!(music_hoard.add_qobuz_url(&artist_id_2, QOBUZ).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Adding URL to artist. + assert!(music_hoard.add_qobuz_url(&artist_id, QOBUZ).is_ok()); + _ = expected.insert(Qobuz::new(QOBUZ).unwrap()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Adding further URLs is an error. + assert!(music_hoard.add_qobuz_url(&artist_id, QOBUZ).is_err()); + assert!(music_hoard.add_qobuz_url(&artist_id, QOBUZ_2).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Removing a URL from an artist not in the collection is an error. + assert!(music_hoard.remove_qobuz_url(&artist_id_2, QOBUZ).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Removing a URL is only okay if it matches the stored one. + assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ_2).is_err()); + assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ).is_ok()); + _ = expected.take(); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Removing a URl if one does not exist is an error. + assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Setting an incorrect URL is an error. + assert!(music_hoard.set_qobuz_url(&artist_id, MUSICBRAINZ).is_err()); + assert!(music_hoard.set_qobuz_url(&artist_id, MUSICBUTLER).is_err()); + assert!(music_hoard.set_qobuz_url(&artist_id, BANDCAMP).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Setting a URL on an artist not in the collection is an error. + assert!(music_hoard.set_qobuz_url(&artist_id_2, QOBUZ).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Setting a URL on an artist. + assert!(music_hoard.set_qobuz_url(&artist_id, QOBUZ).is_ok()); + _ = expected.insert(Qobuz::new(QOBUZ).unwrap()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + assert!(music_hoard.set_qobuz_url(&artist_id, QOBUZ).is_ok()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + assert!(music_hoard.set_qobuz_url(&artist_id, QOBUZ_2).is_ok()); + _ = expected.insert(Qobuz::new(QOBUZ_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Clearing URLs on an artist that does not exist is an error. + assert!(music_hoard.clear_qobuz_url(&artist_id_2).is_err()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Clearing URLs. + assert!(music_hoard.clear_qobuz_url(&artist_id).is_ok()); + _ = expected.take(); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + assert!(music_hoard.delete_artist(&artist_id).is_ok()); } #[test] -- 2.45.2 From e1306e089a46ee1a66cef74edcff7d0e96d01c12 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Tue, 9 Jan 2024 23:49:49 +0100 Subject: [PATCH 09/16] Fix clippy --- src/lib.rs | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b00757b..348e694 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1068,17 +1068,16 @@ mod tests { use super::*; - static MUSICBRAINZ: &'static str = + static MUSICBRAINZ: &str = "https://musicbrainz.org/artist/d368baa8-21ca-4759-9731-0b2753071ad8"; - static MUSICBRAINZ_2: &'static str = + static MUSICBRAINZ_2: &str = "https://musicbrainz.org/artist/823869a5-5ded-4f6b-9fb7-2a9344d83c6b"; - static MUSICBUTLER: &'static str = "https://www.musicbutler.io/artist-page/483340948"; - static MUSICBUTLER_2: &'static str = "https://www.musicbutler.io/artist-page/658903042/"; - static BANDCAMP: &'static str = "https://thelasthangmen.bandcamp.com/"; - static BANDCAMP_2: &'static str = "https://viciouscrusade.bandcamp.com/"; - static QOBUZ: &'static str = "https://www.qobuz.com/nl-nl/interpreter/the-last-hangmen/1244413"; - static QOBUZ_2: &'static str = - "https://www.qobuz.com/nl-nl/interpreter/vicious-crusade/7522386"; + static MUSICBUTLER: &str = "https://www.musicbutler.io/artist-page/483340948"; + static MUSICBUTLER_2: &str = "https://www.musicbutler.io/artist-page/658903042/"; + static BANDCAMP: &str = "https://thelasthangmen.bandcamp.com/"; + static BANDCAMP_2: &str = "https://viciouscrusade.bandcamp.com/"; + static QOBUZ: &str = "https://www.qobuz.com/nl-nl/interpreter/the-last-hangmen/1244413"; + static QOBUZ_2: &str = "https://www.qobuz.com/nl-nl/interpreter/vicious-crusade/7522386"; pub static COLLECTION: Lazy> = Lazy::new(|| collection!()); -- 2.45.2 From 44675c9cbede71d4cd51979938a2bac31c51f157 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 09:16:18 +0100 Subject: [PATCH 10/16] Do not error as much --- Cargo.lock | 12 +- Cargo.toml | 1 - src/lib.rs | 391 +++++++++++++++++++++++++++++------------------------ 3 files changed, 212 insertions(+), 192 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 890b6ad..491d365 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -273,15 +273,6 @@ dependencies = [ "either", ] -[[package]] -name = "itertools" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "25db6b064527c5d482d0423354fcd07a89a2dfe07b67892e62411946db7f07b0" -dependencies = [ - "either", -] - [[package]] name = "itoa" version = "1.0.6" @@ -375,7 +366,6 @@ name = "musichoard" version = "0.1.0" dependencies = [ "crossterm", - "itertools 0.12.0", "mockall", "once_cell", "openssh", @@ -486,7 +476,7 @@ checksum = "59230a63c37f3e18569bdb90e4a89cbf5bf8b06fea0b84e65ea10cc4df47addd" dependencies = [ "difflib", "float-cmp", - "itertools 0.10.5", + "itertools", "normalize-line-endings", "predicates-core", "regex", diff --git a/Cargo.toml b/Cargo.toml index e9d6add..395c326 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,6 @@ edition = "2021" [dependencies] crossterm = { version = "0.26.1", optional = true} -itertools = { version = "0.12.0" } openssh = { version = "0.9.9", features = ["native-mux"], default-features = false, optional = true} ratatui = { version = "0.20.1", optional = true} serde = { version = "1.0.159", features = ["derive"] } diff --git a/src/lib.rs b/src/lib.rs index 348e694..d6410d7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,7 +12,6 @@ use std::{ }; use database::IDatabase; -use itertools::Itertools; use library::{ILibrary, Item, Query}; use serde::{Deserialize, Serialize}; use url::Url; @@ -88,9 +87,9 @@ impl TryFrom<&str> for MusicBrainz { } } -impl AsRef for MusicBrainz { - fn as_ref(&self) -> &str { - self.0.as_str() +impl fmt::Display for MusicBrainz { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) } } @@ -126,6 +125,10 @@ impl MusicButler { Ok(MusicButler(url)) } + pub fn as_str(&self) -> &str { + self.0.as_str() + } + fn invalid_url_error>(url: S) -> InvalidUrlError { InvalidUrlError { url_type: UrlType::MusicButler, @@ -142,12 +145,6 @@ impl TryFrom<&str> for MusicButler { } } -impl AsRef for MusicButler { - fn as_ref(&self) -> &str { - self.0.as_str() - } -} - impl IUrl for MusicButler { fn url(&self) -> &str { self.0.as_str() @@ -173,6 +170,10 @@ impl Bandcamp { Ok(Bandcamp(url)) } + pub fn as_str(&self) -> &str { + self.0.as_str() + } + fn invalid_url_error>(url: S) -> InvalidUrlError { InvalidUrlError { url_type: UrlType::Bandcamp, @@ -189,12 +190,6 @@ impl TryFrom<&str> for Bandcamp { } } -impl AsRef for Bandcamp { - fn as_ref(&self) -> &str { - self.0.as_str() - } -} - impl IUrl for Bandcamp { fn url(&self) -> &str { self.0.as_str() @@ -236,9 +231,9 @@ impl TryFrom<&str> for Qobuz { } } -impl AsRef for Qobuz { - fn as_ref(&self) -> &str { - self.0.as_str() +impl fmt::Display for Qobuz { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", self.0) } } @@ -383,13 +378,13 @@ pub struct Artist { } macro_rules! artist_unique_url_dispatch { - ($add:ident, $remove:ident, $set:ident, $clear:ident, $label:literal, $field:ident) => { + ($add:ident, $remove:ident, $set:ident, $clear:ident, $field:ident) => { fn $add>(&mut self, url: S) -> Result<(), Error> { - Self::add_unique_url(&self.id, $label, &mut self.properties.$field, url) + Self::add_unique_url(&mut self.properties.$field, url) } fn $remove>(&mut self, url: S) -> Result<(), Error> { - Self::remove_unique_url(&self.id, $label, &mut self.properties.$field, url) + Self::remove_unique_url(&mut self.properties.$field, url) } fn $set>(&mut self, url: S) -> Result<(), Error> { @@ -403,13 +398,13 @@ macro_rules! artist_unique_url_dispatch { } macro_rules! artist_multi_url_dispatch { - ($add:ident, $remove:ident, $set:ident, $clear:ident, $label:literal, $field:ident) => { + ($add:ident, $remove:ident, $set:ident, $clear:ident, $field:ident) => { fn $add>(&mut self, urls: Vec) -> Result<(), Error> { - Self::add_multi_urls(&self.id, $label, &mut self.properties.$field, urls) + Self::add_multi_urls(&mut self.properties.$field, urls) } fn $remove>(&mut self, urls: Vec) -> Result<(), Error> { - Self::remove_multi_urls(&self.id, $label, &mut self.properties.$field, urls) + Self::remove_multi_urls(&mut self.properties.$field, urls) } fn $set>(&mut self, urls: Vec) -> Result<(), Error> { @@ -433,65 +428,41 @@ impl Artist { fn add_unique_url< S: AsRef, - T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, + T: for<'a> TryFrom<&'a str, Error = Error> + Eq + fmt::Display, >( - artist_id: &ArtistId, - label: &'static str, container: &mut Option, url: S, ) -> Result<(), Error> { let url: T = url.as_ref().try_into()?; - if let Some(current) = container { - if current == &url { - return Err(Error::CollectionError(format!( - "artist '{}' already has this {} URL: {}", - artist_id, - label, - current.as_ref() - ))); - } else { - return Err(Error::CollectionError(format!( - "artist '{}' already has a different {} URL: {}", - artist_id, - label, - current.as_ref() - ))); + match container { + Some(current) => { + if current != &url { + return Err(Error::CollectionError(format!( + "artist already has a different URL: {}", + current + ))); + } + } + None => { + _ = container.insert(url); } } - _ = container.insert(url); Ok(()) } - fn remove_unique_url< - S: AsRef, - T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, - >( - artist_id: &ArtistId, - label: &'static str, + fn remove_unique_url, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>( container: &mut Option, url: S, ) -> Result<(), Error> { - if container.is_none() { - return Err(Error::CollectionError(format!( - "artist '{}' does not have a {} URL", - artist_id, label - ))); + let url: T = url.as_ref().try_into()?; + + if container == &Some(url) { + _ = container.take(); } - let url: T = url.as_ref().try_into()?; - if container.as_ref().unwrap() == &url { - _ = container.take(); - Ok(()) - } else { - Err(Error::CollectionError(format!( - "artist '{}' does not have this {} URL: {}", - artist_id, - label, - url.as_ref(), - ))) - } + Ok(()) } fn set_unique_url, T: for<'a> TryFrom<&'a str, Error = Error>>( @@ -506,59 +477,32 @@ impl Artist { _ = container.take(); } - fn add_multi_urls< - S: AsRef, - T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, - >( - artist_id: &ArtistId, - label: &'static str, + fn add_multi_urls, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>( container: &mut Vec, urls: Vec, ) -> Result<(), Error> { - // Convert into URLs first to facilitate later comparison. - let urls: Result, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect(); - let mut urls = urls?; + let mut new_urls = urls + .iter() + .map(|url| url.as_ref().try_into()) + .filter(|ref res| { + res.as_ref() + .map(|url| !container.contains(url)) + .unwrap_or(true) // Propagate errors. + }) + .collect::, Error>>()?; - // Do not check and insert. First check if any of the provided URLs already exist so that - // the vector remains unchanged in case of failure. - let overlap: Vec<&T> = urls.iter().filter(|url| container.contains(url)).collect(); - if !overlap.is_empty() { - return Err(Error::CollectionError(format!( - "artist '{}' already has these {} URL(s): {}", - artist_id, - label, - overlap.iter().map(|url| url.as_ref()).format(", ") - ))); - } - - container.append(&mut urls); + container.append(&mut new_urls); Ok(()) } - fn remove_multi_urls< - S: AsRef, - T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef, - >( - artist_id: &ArtistId, - label: &'static str, + fn remove_multi_urls, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>( container: &mut Vec, urls: Vec, ) -> Result<(), Error> { - // Convert into URLs first to facilitate later comparison. - let urls: Result, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect(); - let urls = urls?; - - // Do not check and insert. First check if any of the provided URLs already exist so that - // the vector remains unchanged in case of failure. - let difference: Vec<&T> = urls.iter().filter(|url| !container.contains(url)).collect(); - if !difference.is_empty() { - return Err(Error::CollectionError(format!( - "artist '{}' does not have these {} URL(s): {}", - artist_id, - label, - difference.iter().map(|url| url.as_ref()).format(", ") - ))); - } + let urls = urls + .iter() + .map(|url| url.as_ref().try_into()) + .collect::, Error>>()?; container.retain(|url| !urls.contains(url)); Ok(()) @@ -568,8 +512,11 @@ impl Artist { container: &mut Vec, urls: Vec, ) -> Result<(), Error> { - let urls: Result, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect(); - let mut urls = urls?; + let mut urls = urls + .iter() + .map(|url| url.as_ref().try_into()) + .collect::, Error>>()?; + container.clear(); container.append(&mut urls); Ok(()) @@ -584,7 +531,6 @@ impl Artist { remove_musicbrainz_url, set_musicbrainz_url, clear_musicbrainz_url, - "MusicBrainz", musicbrainz ); @@ -593,7 +539,6 @@ impl Artist { remove_musicbutler_urls, set_musicbutler_urls, clear_musicbutler_urls, - "MusicButler", musicbutler ); @@ -602,7 +547,6 @@ impl Artist { remove_bandcamp_urls, set_bandcamp_urls, clear_bandcamp_urls, - "Bandcamp", bandcamp ); @@ -611,7 +555,6 @@ impl Artist { remove_qobuz_url, set_qobuz_url, clear_qobuz_url, - "Qobuz", qobuz ); } @@ -1197,7 +1140,7 @@ mod tests { } #[test] - fn artist_musicbrainz_url() { + fn add_remove_musicbrainz_url() { let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); @@ -1230,10 +1173,13 @@ mod tests { _ = expected.insert(MusicBrainz::new(MUSICBRAINZ).unwrap()); assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); - // Adding further URLs is an error. + // Adding the same URL again is ok, but does not do anything. assert!(music_hoard .add_musicbrainz_url(&artist_id, MUSICBRAINZ) - .is_err()); + .is_ok()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Adding further URLs is an error. assert!(music_hoard .add_musicbrainz_url(&artist_id, MUSICBRAINZ_2) .is_err()); @@ -1245,20 +1191,34 @@ mod tests { .is_err()); assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); - // Removing a URL is only okay if it matches the stored one. + // Removing a URL not in the collection is okay, but does not do anything. assert!(music_hoard .remove_musicbrainz_url(&artist_id, MUSICBRAINZ_2) - .is_err()); + .is_ok()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + + // Removing a URL in the collection removes it. assert!(music_hoard .remove_musicbrainz_url(&artist_id, MUSICBRAINZ) .is_ok()); _ = expected.take(); assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); - // Removing a URl if one does not exist is an error. assert!(music_hoard .remove_musicbrainz_url(&artist_id, MUSICBRAINZ) - .is_err()); + .is_ok()); + assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); + } + + #[test] + fn set_clear_musicbrainz_url() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); // Setting an incorrect URL is an error. @@ -1308,7 +1268,7 @@ mod tests { } #[test] - fn artist_musicbutler_urls() { + fn add_remove_musicbutler_urls() { let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); @@ -1339,20 +1299,20 @@ mod tests { .is_err()); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - // Adding a URL. + // Adding a single URL. assert!(music_hoard .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) .is_ok()); expected.push(MusicButler::new(MUSICBUTLER).unwrap()); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - // Adding a URL that already exists is an error. + // Adding a URL that already exists is ok, but does not do anything. assert!(music_hoard .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - // Adding another URL. + // Adding another single URL. assert!(music_hoard .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) .is_ok()); @@ -1361,10 +1321,10 @@ mod tests { assert!(music_hoard .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - // Removing URLs to an artist not in the collection is an error. + // Removing URLs from an artist not in the collection is an error. assert!(music_hoard .remove_musicbutler_urls(&artist_id_2, vec![MUSICBUTLER]) .is_err()); @@ -1374,39 +1334,54 @@ mod tests { assert!(music_hoard .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) .is_ok()); - expected.retain(|url| url.as_ref() != MUSICBUTLER); + expected.retain(|url| url.as_str() != MUSICBUTLER); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - // If any URL does not exist removing URLs is an error. + // Removing URls that do not exist is okay, they will be ignored. assert!(music_hoard .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) - .is_err()); - assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - - assert!(music_hoard - .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) - .is_err()); - assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - - // If any URL already exists exists adding URLs is an error. - assert!(music_hoard - .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); // Removing a URL. assert!(music_hoard .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) .is_ok()); - expected.retain(|url| url.as_ref() != MUSICBUTLER_2); + expected.retain(|url| url.as_str() != MUSICBUTLER_2); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); assert!(music_hoard .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER_2]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - // Adding mutliple URLs is okay if none of them already exist. + // Adding URLs if some exist is okay, they will be ignored. + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) + .is_ok()); + expected.push(MusicButler::new(MUSICBUTLER).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + assert!(music_hoard + .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_ok()); + expected.push(MusicButler::new(MUSICBUTLER_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Removing URLs if some do not exist is okay, they will be ignored. + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER]) + .is_ok()); + expected.retain(|url| url.as_str() != MUSICBUTLER); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + assert!(music_hoard + .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) + .is_ok()); + expected.retain(|url| url.as_str() != MUSICBUTLER_2); + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + + // Adding mutliple URLs without clashes. assert!(music_hoard .add_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) .is_ok()); @@ -1414,12 +1389,24 @@ mod tests { expected.push(MusicButler::new(MUSICBUTLER_2).unwrap()); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); - // Removing multiple URLs is okay if they all exist. + // Removing multiple URLs without clashes. assert!(music_hoard .remove_musicbutler_urls(&artist_id, vec![MUSICBUTLER, MUSICBUTLER_2]) .is_ok()); expected.clear(); assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); + } + + #[test] + fn set_clear_musicbutler_urls() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Vec = vec![]; + assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); // If any URL is incorrect setting URLs is an error. assert!(music_hoard @@ -1474,7 +1461,7 @@ mod tests { } #[test] - fn artist_bandcamp_urls() { + fn add_remove_bandcamp_urls() { let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); @@ -1505,20 +1492,20 @@ mod tests { .is_err()); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - // Adding a URL. + // Adding a single URL. assert!(music_hoard .add_bandcamp_urls(&artist_id, vec![BANDCAMP]) .is_ok()); expected.push(Bandcamp::new(BANDCAMP).unwrap()); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - // Adding a URL that already exists is an error. + // Adding a URL that already exists is ok, but does not do anything. assert!(music_hoard .add_bandcamp_urls(&artist_id, vec![BANDCAMP]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - // Adding another URL. + // Adding another single URL. assert!(music_hoard .add_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) .is_ok()); @@ -1527,10 +1514,10 @@ mod tests { assert!(music_hoard .add_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - // Removing URLs to an artist not in the collection is an error. + // Removing URLs from an artist not in the collection is an error. assert!(music_hoard .remove_bandcamp_urls(&artist_id_2, vec![BANDCAMP]) .is_err()); @@ -1540,39 +1527,54 @@ mod tests { assert!(music_hoard .remove_bandcamp_urls(&artist_id, vec![BANDCAMP]) .is_ok()); - expected.retain(|url| url.as_ref() != BANDCAMP); + expected.retain(|url| url.as_str() != BANDCAMP); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - // If any URL does not exist removing URLs is an error. + // Removing URls that do not exist is okay, they will be ignored. assert!(music_hoard .remove_bandcamp_urls(&artist_id, vec![BANDCAMP]) - .is_err()); - assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - - assert!(music_hoard - .remove_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) - .is_err()); - assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - - // If any URL already exists exists adding URLs is an error. - assert!(music_hoard - .add_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); // Removing a URL. assert!(music_hoard .remove_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) .is_ok()); - expected.retain(|url| url.as_ref() != BANDCAMP_2); + expected.retain(|url| url.as_str() != BANDCAMP_2); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); assert!(music_hoard .remove_bandcamp_urls(&artist_id, vec![BANDCAMP_2]) - .is_err()); + .is_ok()); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - // Adding mutliple URLs is okay if none of them already exist. + // Adding URLs if some exist is okay, they will be ignored. + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP]) + .is_ok()); + expected.push(Bandcamp::new(BANDCAMP).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + assert!(music_hoard + .add_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_ok()); + expected.push(Bandcamp::new(BANDCAMP_2).unwrap()); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Removing URLs if some do not exist is okay, they will be ignored. + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP]) + .is_ok()); + expected.retain(|url| url.as_str() != BANDCAMP); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + assert!(music_hoard + .remove_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) + .is_ok()); + expected.retain(|url| url.as_str() != BANDCAMP_2); + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + + // Adding mutliple URLs without clashes. assert!(music_hoard .add_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) .is_ok()); @@ -1580,12 +1582,24 @@ mod tests { expected.push(Bandcamp::new(BANDCAMP_2).unwrap()); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); - // Removing multiple URLs is okay if they all exist. + // Removing multiple URLs without clashes. assert!(music_hoard .remove_bandcamp_urls(&artist_id, vec![BANDCAMP, BANDCAMP_2]) .is_ok()); expected.clear(); assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); + } + + #[test] + fn set_clear_bandcamp_urls() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Vec = vec![]; + assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); // If any URL is incorrect setting URLs is an error. assert!(music_hoard @@ -1640,7 +1654,7 @@ mod tests { } #[test] - fn artist_qobuz_url() { + fn add_remove_qobuz_url() { let artist_id = ArtistId::new("an artist"); let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); @@ -1665,8 +1679,11 @@ mod tests { _ = expected.insert(Qobuz::new(QOBUZ).unwrap()); assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + // Adding the same URL again is ok, but does not do anything. + assert!(music_hoard.add_qobuz_url(&artist_id, QOBUZ).is_ok()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + // Adding further URLs is an error. - assert!(music_hoard.add_qobuz_url(&artist_id, QOBUZ).is_err()); assert!(music_hoard.add_qobuz_url(&artist_id, QOBUZ_2).is_err()); assert_eq!(music_hoard.collection[0].properties.qobuz, expected); @@ -1674,20 +1691,34 @@ mod tests { assert!(music_hoard.remove_qobuz_url(&artist_id_2, QOBUZ).is_err()); assert_eq!(music_hoard.collection[0].properties.qobuz, expected); - // Removing a URL is only okay if it matches the stored one. - assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ_2).is_err()); + // Removing a URL not in the collection is okay, but does not do anything. + assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ_2).is_ok()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + + // Removing a URL in the collection removes it. assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ).is_ok()); _ = expected.take(); assert_eq!(music_hoard.collection[0].properties.qobuz, expected); - // Removing a URl if one does not exist is an error. - assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ).is_err()); + assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ).is_ok()); + assert_eq!(music_hoard.collection[0].properties.qobuz, expected); + } + + #[test] + fn set_clear_qobuz_url() { + let artist_id = ArtistId::new("an artist"); + let artist_id_2 = ArtistId::new("another artist"); + let mut music_hoard = MusicHoard::::new(None, None); + + assert!(music_hoard.new_artist(&artist_id).is_ok()); + + let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.qobuz, expected); // Setting an incorrect URL is an error. - assert!(music_hoard.set_qobuz_url(&artist_id, MUSICBRAINZ).is_err()); assert!(music_hoard.set_qobuz_url(&artist_id, MUSICBUTLER).is_err()); assert!(music_hoard.set_qobuz_url(&artist_id, BANDCAMP).is_err()); + assert!(music_hoard.set_qobuz_url(&artist_id, MUSICBRAINZ).is_err()); assert_eq!(music_hoard.collection[0].properties.qobuz, expected); // Setting a URL on an artist not in the collection is an error. -- 2.45.2 From d0e04d8a78f2afe2fc0599b96cdd8e0297d01000 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 09:17:15 +0100 Subject: [PATCH 11/16] Fix clippy --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index d6410d7..523993c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -484,7 +484,7 @@ impl Artist { let mut new_urls = urls .iter() .map(|url| url.as_ref().try_into()) - .filter(|ref res| { + .filter(|res| { res.as_ref() .map(|url| !container.contains(url)) .unwrap_or(true) // Propagate errors. -- 2.45.2 From f36247f53b05380b75dd05c54df12bb548b64452 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 18:58:29 +0100 Subject: [PATCH 12/16] Auto-generate the method identifiers --- Cargo.lock | 7 ++ Cargo.toml | 1 + src/bin/mh-edit.rs | 89 ++++++++---------- src/lib.rs | 222 ++++++++++++++++++++------------------------- 4 files changed, 140 insertions(+), 179 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 491d365..4ad4190 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -369,6 +369,7 @@ dependencies = [ "mockall", "once_cell", "openssh", + "paste", "ratatui", "serde", "serde_json", @@ -456,6 +457,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "paste" +version = "1.0.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c" + [[package]] name = "percent-encoding" version = "2.2.0" diff --git a/Cargo.toml b/Cargo.toml index 395c326..9fe2d21 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" [dependencies] crossterm = { version = "0.26.1", optional = true} openssh = { version = "0.9.9", features = ["native-mux"], default-features = false, optional = true} +paste = { version = "1.0.14" } ratatui = { version = "0.20.1", optional = true} serde = { version = "1.0.159", features = ["derive"] } serde_json = { version = "1.0.95", optional = true} diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index 205fb13..5b717f3 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -1,3 +1,4 @@ +use paste::paste; use std::path::PathBuf; use structopt::StructOpt; @@ -73,37 +74,39 @@ struct MultiUrlValue { } macro_rules! url_command_dispatch { - ($cmd:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident, $url:ident) => { - match $cmd { - UrlCommand::Add(url_value) => { - $mh.$add(ArtistId::new(url_value.artist), url_value.$url) - .expect("failed to add URL(s)"); - } - UrlCommand::Remove(url_value) => { - $mh.$remove(ArtistId::new(url_value.artist), url_value.$url) - .expect("failed to remove URL(s)"); - } - UrlCommand::Set(url_value) => { - $mh.$set(ArtistId::new(url_value.artist), url_value.$url) - .expect("failed to set URL(s)"); - } - UrlCommand::Clear(artist_value) => { - $mh.$clear(ArtistId::new(artist_value.artist)) - .expect("failed to clear URL(s)"); + ($cmd:ident, $mh:ident, $field:ident, $url:ident) => { + paste! { + match $cmd { + UrlCommand::Add(url_value) => { + $mh.[](ArtistId::new(url_value.artist), url_value.$url) + .expect("failed to add URL(s)"); + } + UrlCommand::Remove(url_value) => { + $mh.[](ArtistId::new(url_value.artist), url_value.$url) + .expect("failed to remove URL(s)"); + } + UrlCommand::Set(url_value) => { + $mh.[](ArtistId::new(url_value.artist), url_value.$url) + .expect("failed to set URL(s)"); + } + UrlCommand::Clear(artist_value) => { + $mh.[](ArtistId::new(artist_value.artist)) + .expect("failed to clear URL(s)"); + } } } }; } macro_rules! single_url_command_dispatch { - ($cmd:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident) => { - url_command_dispatch!($cmd, $mh, $add, $remove, $set, $clear, url) + ($cmd:ident, $mh:ident, $field:ident) => { + url_command_dispatch!($cmd, $mh, $field, url) }; } macro_rules! multi_url_command_dispatch { - ($cmd:ident, $mh:ident, $add:ident, $remove:ident, $set:ident, $clear:ident) => { - url_command_dispatch!($cmd, $mh, $add, $remove, $set, $clear, urls) + ($cmd:ident, $mh:ident, $field:ident) => { + url_command_dispatch!($cmd, $mh, $field, urls) }; } @@ -120,38 +123,18 @@ impl ArtistCommand { .delete_artist(ArtistId::new(artist_value.artist)) .expect("failed to delete artist"); } - ArtistCommand::MusicBrainz(url_command) => single_url_command_dispatch!( - url_command, - music_hoard, - add_musicbrainz_url, - remove_musicbrainz_url, - set_musicbrainz_url, - clear_musicbrainz_url - ), - ArtistCommand::MusicButler(url_command) => multi_url_command_dispatch!( - url_command, - music_hoard, - add_musicbutler_urls, - remove_musicbutler_urls, - set_musicbutler_urls, - clear_musicbutler_urls - ), - ArtistCommand::Bandcamp(url_command) => multi_url_command_dispatch!( - url_command, - music_hoard, - add_bandcamp_urls, - remove_bandcamp_urls, - set_bandcamp_urls, - clear_bandcamp_urls - ), - ArtistCommand::Qobuz(url_command) => single_url_command_dispatch!( - url_command, - music_hoard, - add_qobuz_url, - remove_qobuz_url, - set_qobuz_url, - clear_qobuz_url - ), + ArtistCommand::MusicBrainz(url_command) => { + single_url_command_dispatch!(url_command, music_hoard, musicbrainz) + } + ArtistCommand::MusicButler(url_command) => { + multi_url_command_dispatch!(url_command, music_hoard, musicbutler) + } + ArtistCommand::Bandcamp(url_command) => { + multi_url_command_dispatch!(url_command, music_hoard, bandcamp) + } + ArtistCommand::Qobuz(url_command) => { + single_url_command_dispatch!(url_command, music_hoard, qobuz) + } } } } diff --git a/src/lib.rs b/src/lib.rs index 523993c..7131ff5 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,6 +13,7 @@ use std::{ use database::IDatabase; use library::{ILibrary, Item, Query}; +use paste::paste; use serde::{Deserialize, Serialize}; use url::Url; use uuid::Uuid; @@ -378,41 +379,45 @@ pub struct Artist { } macro_rules! artist_unique_url_dispatch { - ($add:ident, $remove:ident, $set:ident, $clear:ident, $field:ident) => { - fn $add>(&mut self, url: S) -> Result<(), Error> { - Self::add_unique_url(&mut self.properties.$field, url) - } + ($field:ident) => { + paste! { + fn []>(&mut self, url: S) -> Result<(), Error> { + Self::add_unique_url(&mut self.properties.$field, url) + } - fn $remove>(&mut self, url: S) -> Result<(), Error> { - Self::remove_unique_url(&mut self.properties.$field, url) - } + fn []>(&mut self, url: S) -> Result<(), Error> { + Self::remove_unique_url(&mut self.properties.$field, url) + } - fn $set>(&mut self, url: S) -> Result<(), Error> { - Self::set_unique_url(&mut self.properties.$field, url) - } + fn []>(&mut self, url: S) -> Result<(), Error> { + Self::set_unique_url(&mut self.properties.$field, url) + } - fn $clear(&mut self) { - Self::clear_unique_url(&mut self.properties.$field); + fn [](&mut self) { + Self::clear_unique_url(&mut self.properties.$field); + } } }; } macro_rules! artist_multi_url_dispatch { - ($add:ident, $remove:ident, $set:ident, $clear:ident, $field:ident) => { - fn $add>(&mut self, urls: Vec) -> Result<(), Error> { - Self::add_multi_urls(&mut self.properties.$field, urls) - } + ($field:ident) => { + paste! { + fn []>(&mut self, urls: Vec) -> Result<(), Error> { + Self::add_multi_urls(&mut self.properties.$field, urls) + } - fn $remove>(&mut self, urls: Vec) -> Result<(), Error> { - Self::remove_multi_urls(&mut self.properties.$field, urls) - } + fn []>(&mut self, urls: Vec) -> Result<(), Error> { + Self::remove_multi_urls(&mut self.properties.$field, urls) + } - fn $set>(&mut self, urls: Vec) -> Result<(), Error> { - Self::set_multi_urls(&mut self.properties.$field, urls) - } + fn []>(&mut self, urls: Vec) -> Result<(), Error> { + Self::set_multi_urls(&mut self.properties.$field, urls) + } - fn $clear(&mut self) { - Self::clear_multi_urls(&mut self.properties.$field); + fn [](&mut self) { + Self::clear_multi_urls(&mut self.properties.$field); + } } }; } @@ -526,37 +531,13 @@ impl Artist { container.clear(); } - artist_unique_url_dispatch!( - add_musicbrainz_url, - remove_musicbrainz_url, - set_musicbrainz_url, - clear_musicbrainz_url, - musicbrainz - ); + artist_unique_url_dispatch!(musicbrainz); - artist_multi_url_dispatch!( - add_musicbutler_urls, - remove_musicbutler_urls, - set_musicbutler_urls, - clear_musicbutler_urls, - musicbutler - ); + artist_multi_url_dispatch!(musicbutler); - artist_multi_url_dispatch!( - add_bandcamp_urls, - remove_bandcamp_urls, - set_bandcamp_urls, - clear_bandcamp_urls, - bandcamp - ); + artist_multi_url_dispatch!(bandcamp); - artist_unique_url_dispatch!( - add_qobuz_url, - remove_qobuz_url, - set_qobuz_url, - clear_qobuz_url, - qobuz - ); + artist_unique_url_dispatch!(qobuz); } impl PartialOrd for Artist { @@ -721,67 +702,76 @@ pub struct MusicHoard { } macro_rules! music_hoard_unique_url_dispatch { - ($add:ident, $remove:ident, $set:ident, $clear:ident) => { - pub fn $add, S: AsRef>( - &mut self, - artist_id: ID, - url: S, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$add(url) - } + ($field:ident) => { + paste! { + pub fn [], S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](url) + } - pub fn $remove, S: AsRef>( - &mut self, - artist_id: ID, - url: S, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$remove(url) - } + pub fn [], S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](url) + } - pub fn $set, S: AsRef>( - &mut self, - artist_id: ID, - url: S, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$set(url) - } + pub fn [], S: AsRef>( + &mut self, + artist_id: ID, + url: S, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](url) + } - pub fn $clear>(&mut self, artist_id: ID) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$clear(); - Ok(()) + pub fn []>( + &mut self, + artist_id: ID, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](); + Ok(()) + } } }; } macro_rules! music_hoard_multi_url_dispatch { - ($add:ident, $remove:ident, $set:ident, $clear:ident) => { - pub fn $add, S: AsRef>( - &mut self, - artist_id: ID, - urls: Vec, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$add(urls) - } + ($field:ident) => { + paste! { + pub fn [], S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](urls) + } - pub fn $remove, S: AsRef>( - &mut self, - artist_id: ID, - urls: Vec, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$remove(urls) - } + pub fn [], S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](urls) + } - pub fn $set, S: AsRef>( - &mut self, - artist_id: ID, - urls: Vec, - ) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$set(urls) - } + pub fn [], S: AsRef>( + &mut self, + artist_id: ID, + urls: Vec, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](urls) + } - pub fn $clear>(&mut self, artist_id: ID) -> Result<(), Error> { - self.get_artist_or_err(artist_id.as_ref())?.$clear(); - Ok(()) + pub fn []>( + &mut self, artist_id: ID, + ) -> Result<(), Error> { + self.get_artist_or_err(artist_id.as_ref())?.[](); + Ok(()) + } } }; } @@ -876,33 +866,13 @@ impl MusicHoard { } } - music_hoard_unique_url_dispatch!( - add_musicbrainz_url, - remove_musicbrainz_url, - set_musicbrainz_url, - clear_musicbrainz_url - ); + music_hoard_unique_url_dispatch!(musicbrainz); - music_hoard_multi_url_dispatch!( - add_musicbutler_urls, - remove_musicbutler_urls, - set_musicbutler_urls, - clear_musicbutler_urls - ); + music_hoard_multi_url_dispatch!(musicbutler); - music_hoard_multi_url_dispatch!( - add_bandcamp_urls, - remove_bandcamp_urls, - set_bandcamp_urls, - clear_bandcamp_urls - ); + music_hoard_multi_url_dispatch!(bandcamp); - music_hoard_unique_url_dispatch!( - add_qobuz_url, - remove_qobuz_url, - set_qobuz_url, - clear_qobuz_url - ); + music_hoard_unique_url_dispatch!(qobuz); fn sort(collection: &mut [Artist]) { collection.sort_unstable(); -- 2.45.2 From b35a8eea0a13f063d3c79564aa229b276429ca24 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 19:01:37 +0100 Subject: [PATCH 13/16] Make tests consistent --- src/lib.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 7131ff5..98627bd 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1233,8 +1233,6 @@ mod tests { assert!(music_hoard.clear_musicbrainz_url(&artist_id).is_ok()); _ = expected.take(); assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); - - assert!(music_hoard.delete_artist(&artist_id).is_ok()); } #[test] @@ -1715,8 +1713,6 @@ mod tests { assert!(music_hoard.clear_qobuz_url(&artist_id).is_ok()); _ = expected.take(); assert_eq!(music_hoard.collection[0].properties.qobuz, expected); - - assert!(music_hoard.delete_artist(&artist_id).is_ok()); } #[test] -- 2.45.2 From 3a266c0ec534c43dbc565cf89d0023722add3961 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 19:30:31 +0100 Subject: [PATCH 14/16] Add docstrings to the mh-edit command --- src/bin/mh-edit.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index 5b717f3..6025ba2 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -1,6 +1,6 @@ use paste::paste; use std::path::PathBuf; -use structopt::StructOpt; +use structopt::{clap::AppSettings, StructOpt}; use musichoard::{ database::json::{backend::JsonDatabaseFileBackend, JsonDatabase}, @@ -11,20 +11,23 @@ use musichoard::{ type MH = MusicHoard>; #[derive(StructOpt, Debug)] +#[structopt(about = "mh-edit: edit the MusicHoard database", + global_settings=&[AppSettings::DeriveDisplayOrder])] struct Opt { - #[structopt(subcommand)] - category: Category, - #[structopt( long = "database", name = "database file path", default_value = "database.json" )] database_file_path: PathBuf, + + #[structopt(subcommand)] + category: Category, } #[derive(StructOpt, Debug)] enum Category { + #[structopt(about = "Edit artist information")] Artist(ArtistCommand), } @@ -38,38 +41,51 @@ impl Category { #[derive(StructOpt, Debug)] enum ArtistCommand { + #[structopt(about = "Add a new artist to the collection")] New(ArtistValue), + #[structopt(about = "Delete an artist from the collection")] Delete(ArtistValue), - #[structopt(name = "musicbrainz")] + #[structopt(name = "musicbrainz", about = "Edit the MusicBrainz URL of an artist")] MusicBrainz(UrlCommand), - #[structopt(name = "musicbutler")] + #[structopt(name = "musicbutler", about = "Edit the MusicButler URL of an artist")] MusicButler(UrlCommand), + #[structopt(about = "Edit the Bandcamp URL of an artist")] Bandcamp(UrlCommand), + #[structopt(about = "Edit the Qobuz URL of an artist")] Qobuz(UrlCommand), } #[derive(StructOpt, Debug)] struct ArtistValue { + #[structopt(help = "The name of the artist")] artist: String, } #[derive(StructOpt, Debug)] enum UrlCommand { + #[structopt(about = "Add URL(s) without overwriting existing values")] Add(T), + #[structopt(about = "Remove the provided URL(s)")] Remove(T), + #[structopt(about = "Set the provided URL(s) overwriting any previous values")] Set(T), + #[structopt(about = "Clear all URL(s)")] Clear(ArtistValue), } #[derive(StructOpt, Debug)] struct SingleUrlValue { + #[structopt(help = "The name of the artist")] artist: String, + #[structopt(help = "The URL")] url: String, } #[derive(StructOpt, Debug)] struct MultiUrlValue { + #[structopt(help = "The name of the artist")] artist: String, + #[structopt(help = "The list of URLs")] urls: Vec, } -- 2.45.2 From b2ba9fdc4cd6320f70c16f6f9c8b588490ad16dc Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 19:34:20 +0100 Subject: [PATCH 15/16] Clearer about messages --- src/bin/mh-edit.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index 6025ba2..aaaf61a 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -47,9 +47,9 @@ enum ArtistCommand { Delete(ArtistValue), #[structopt(name = "musicbrainz", about = "Edit the MusicBrainz URL of an artist")] MusicBrainz(UrlCommand), - #[structopt(name = "musicbutler", about = "Edit the MusicButler URL of an artist")] + #[structopt(name = "musicbutler", about = "Edit the MusicButler URL(s) of an artist")] MusicButler(UrlCommand), - #[structopt(about = "Edit the Bandcamp URL of an artist")] + #[structopt(about = "Edit the Bandcamp URL(s) of an artist")] Bandcamp(UrlCommand), #[structopt(about = "Edit the Qobuz URL of an artist")] Qobuz(UrlCommand), @@ -63,11 +63,11 @@ struct ArtistValue { #[derive(StructOpt, Debug)] enum UrlCommand { - #[structopt(about = "Add URL(s) without overwriting existing values")] + #[structopt(about = "Add the provided URL(s) without overwriting existing values")] Add(T), #[structopt(about = "Remove the provided URL(s)")] Remove(T), - #[structopt(about = "Set the provided URL(s) overwriting any previous values")] + #[structopt(about = "Set the provided URL(s) overwriting any existing values")] Set(T), #[structopt(about = "Clear all URL(s)")] Clear(ArtistValue), -- 2.45.2 From 0a63ac46190cbd084428c70ecd311bb1bcbc7ce2 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 20:30:29 +0100 Subject: [PATCH 16/16] Some clean up --- src/bin/mh-edit.rs | 5 ++++- src/lib.rs | 44 +++++++++++++++++++++----------------------- 2 files changed, 25 insertions(+), 24 deletions(-) diff --git a/src/bin/mh-edit.rs b/src/bin/mh-edit.rs index aaaf61a..daef105 100644 --- a/src/bin/mh-edit.rs +++ b/src/bin/mh-edit.rs @@ -47,7 +47,10 @@ enum ArtistCommand { Delete(ArtistValue), #[structopt(name = "musicbrainz", about = "Edit the MusicBrainz URL of an artist")] MusicBrainz(UrlCommand), - #[structopt(name = "musicbutler", about = "Edit the MusicButler URL(s) of an artist")] + #[structopt( + name = "musicbutler", + about = "Edit the MusicButler URL(s) of an artist" + )] MusicButler(UrlCommand), #[structopt(about = "Edit the Bandcamp URL(s) of an artist")] Bandcamp(UrlCommand), diff --git a/src/lib.rs b/src/lib.rs index 98627bd..5d95931 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,7 +6,7 @@ pub mod library; use std::{ cmp::Ordering, collections::{HashMap, HashSet}, - fmt::{self, Debug}, + fmt::{self, Debug, Display}, iter::Peekable, mem, }; @@ -42,7 +42,7 @@ struct InvalidUrlError { url: String, } -impl fmt::Display for InvalidUrlError { +impl Display for InvalidUrlError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "invalid url of type {:?}: {}", self.url_type, self.url) } @@ -88,7 +88,7 @@ impl TryFrom<&str> for MusicBrainz { } } -impl fmt::Display for MusicBrainz { +impl Display for MusicBrainz { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.0) } @@ -232,7 +232,7 @@ impl TryFrom<&str> for Qobuz { } } -impl fmt::Display for Qobuz { +impl Display for Qobuz { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.0) } @@ -344,7 +344,7 @@ impl ArtistId { } } -impl fmt::Display for ArtistId { +impl Display for ArtistId { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "{}", self.name) } @@ -431,10 +431,7 @@ impl Artist { } } - fn add_unique_url< - S: AsRef, - T: for<'a> TryFrom<&'a str, Error = Error> + Eq + fmt::Display, - >( + fn add_unique_url, T: for<'a> TryFrom<&'a str, Error = Error> + Eq + Display>( container: &mut Option, url: S, ) -> Result<(), Error> { @@ -643,7 +640,7 @@ pub enum Error { InvalidUrlError(String), } -impl fmt::Display for Error { +impl Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { Self::CollectionError(ref s) => write!(f, "failed to read/write the collection: {s}"), @@ -832,15 +829,16 @@ impl MusicHoard { } } - pub fn new_artist>(&mut self, artist_id: ID) -> Result<(), Error> { - if let Ok(artist) = self.get_artist_or_err(artist_id.as_ref()) { + pub fn new_artist>(&mut self, artist_id: ID) -> Result<(), Error> { + let artist_id: ArtistId = artist_id.into(); + if let Ok(artist) = self.get_artist_or_err(&artist_id) { return Err(Error::CollectionError(format!( "artist '{}' is already in the collection", artist.id ))); } - let new_artist = vec![Artist::new(artist_id.as_ref().clone())]; + let new_artist = vec![Artist::new(artist_id)]; let collection = mem::take(&mut self.collection); self.collection = Self::merge(collection, new_artist); @@ -1090,9 +1088,9 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); - let actual_err = music_hoard.new_artist(&artist_id).unwrap_err(); + let actual_err = music_hoard.new_artist(artist_id.clone()).unwrap_err(); let expected_err = Error::CollectionError(String::from( "artist 'an artist' is already in the collection", )); @@ -1115,7 +1113,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); @@ -1186,7 +1184,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.musicbrainz, expected); @@ -1241,7 +1239,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); @@ -1371,7 +1369,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.musicbutler, expected); @@ -1434,7 +1432,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); @@ -1564,7 +1562,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Vec = vec![]; assert_eq!(music_hoard.collection[0].properties.bandcamp, expected); @@ -1627,7 +1625,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.qobuz, expected); @@ -1678,7 +1676,7 @@ mod tests { let artist_id_2 = ArtistId::new("another artist"); let mut music_hoard = MusicHoard::::new(None, None); - assert!(music_hoard.new_artist(&artist_id).is_ok()); + assert!(music_hoard.new_artist(artist_id.clone()).is_ok()); let mut expected: Option = None; assert_eq!(music_hoard.collection[0].properties.qobuz, expected); -- 2.45.2