From 44675c9cbede71d4cd51979938a2bac31c51f157 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Wed, 10 Jan 2024 09:16:18 +0100 Subject: [PATCH] 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.