Do not error as much

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",

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"] }

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<str> for MusicBrainz {
+impl fmt::Display for MusicBrainz {
-    fn as_ref(&self) -> &str {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.0.as_str()
+        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<S: Into<String>>(url: S) -> InvalidUrlError {
         InvalidUrlError {
             url_type: UrlType::MusicButler,
@@ -142,12 +145,6 @@ impl TryFrom<&str> for MusicButler {
     }
 }
 
-impl AsRef<str> 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<S: Into<String>>(url: S) -> InvalidUrlError {
         InvalidUrlError {
             url_type: UrlType::Bandcamp,
@@ -189,12 +190,6 @@ impl TryFrom<&str> for Bandcamp {
     }
 }
 
-impl AsRef<str> 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<str> for Qobuz {
+impl fmt::Display for Qobuz {
-    fn as_ref(&self) -> &str {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        self.0.as_str()
+        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<S: AsRef<str>>(&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<S: AsRef<str>>(&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<S: AsRef<str>>(&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<S: AsRef<str>>(&mut self, urls: Vec<S>) -> 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<S: AsRef<str>>(&mut self, urls: Vec<S>) -> 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<S: AsRef<str>>(&mut self, urls: Vec<S>) -> Result<(), Error> {
@@ -433,65 +428,41 @@ impl Artist {
 
     fn add_unique_url<
         S: AsRef<str>,
-        T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef<str>,
+        T: for<'a> TryFrom<&'a str, Error = Error> + Eq + fmt::Display,
     >(
-        artist_id: &ArtistId,
-        label: &'static str,
         container: &mut Option<T>,
         url: S,
     ) -> Result<(), Error> {
         let url: T = url.as_ref().try_into()?;
 
-        if let Some(current) = container {
+        match container {
-            if current == &url {
+            Some(current) => {
-                return Err(Error::CollectionError(format!(
+                if current != &url {
-                    "artist '{}' already has this {} URL: {}",
+                    return Err(Error::CollectionError(format!(
-                    artist_id,
+                        "artist already has a different URL: {}",
-                    label,
+                        current
-                    current.as_ref()
+                    )));
-                )));
+                }
-            } else {
+            }
-                return Err(Error::CollectionError(format!(
+            None => {
-                    "artist '{}' already has a different {} URL: {}",
+                _ = container.insert(url);
-                    artist_id,
-                    label,
-                    current.as_ref()
-                )));
             }
         }
 
-        _ = container.insert(url);
         Ok(())
     }
 
-    fn remove_unique_url<
+    fn remove_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
-        S: AsRef<str>,
-        T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef<str>,
-    >(
-        artist_id: &ArtistId,
-        label: &'static str,
         container: &mut Option<T>,
         url: S,
     ) -> Result<(), Error> {
-        if container.is_none() {
+        let url: T = url.as_ref().try_into()?;
-            return Err(Error::CollectionError(format!(
+
-                "artist '{}' does not have a {} URL",
+        if container == &Some(url) {
-                artist_id, label
+            _ = container.take();
-            )));
         }
 
-        let url: T = url.as_ref().try_into()?;
+        Ok(())
-        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(),
-            )))
-        }
     }
 
     fn set_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error>>(
@@ -506,59 +477,32 @@ impl Artist {
         _ = container.take();
     }
 
-    fn add_multi_urls<
+    fn add_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
-        S: AsRef<str>,
-        T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef<str>,
-    >(
-        artist_id: &ArtistId,
-        label: &'static str,
         container: &mut Vec<T>,
         urls: Vec<S>,
     ) -> Result<(), Error> {
-        // Convert into URLs first to facilitate later comparison.
+        let mut new_urls = urls
-        let urls: Result<Vec<T>, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect();
+            .iter()
-        let mut urls = urls?;
+            .map(|url| url.as_ref().try_into())
+            .filter(|ref res| {
+                res.as_ref()
+                    .map(|url| !container.contains(url))
+                    .unwrap_or(true) // Propagate errors.
+            })
+            .collect::<Result<Vec<T>, Error>>()?;
 
-        // Do not check and insert. First check if any of the provided URLs already exist so that
+        container.append(&mut new_urls);
-        // 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);
         Ok(())
     }
 
-    fn remove_multi_urls<
+    fn remove_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
-        S: AsRef<str>,
-        T: for<'a> TryFrom<&'a str, Error = Error> + Eq + AsRef<str>,
-    >(
-        artist_id: &ArtistId,
-        label: &'static str,
         container: &mut Vec<T>,
         urls: Vec<S>,
     ) -> Result<(), Error> {
-        // Convert into URLs first to facilitate later comparison.
+        let urls = urls
-        let urls: Result<Vec<T>, Error> = urls.iter().map(|url| url.as_ref().try_into()).collect();
+            .iter()
-        let urls = urls?;
+            .map(|url| url.as_ref().try_into())
-
+            .collect::<Result<Vec<T>, 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 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(", ")
-            )));
-        }
 
         container.retain(|url| !urls.contains(url));
         Ok(())
@@ -568,8 +512,11 @@ impl Artist {
         container: &mut Vec<T>,
         urls: Vec<S>,
     ) -> Result<(), Error> {
-        let urls: Result<Vec<T>, 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::<Result<Vec<T>, 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::<NoLibrary, NoDatabase>::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::<NoLibrary, NoDatabase>::new(None, None);
+
+        assert!(music_hoard.new_artist(&artist_id).is_ok());
+
+        let mut expected: Option<MusicBrainz> = 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::<NoLibrary, NoDatabase>::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());
+            .is_ok());
-        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);
+        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::<NoLibrary, NoDatabase>::new(None, None);
+
+        assert!(music_hoard.new_artist(&artist_id).is_ok());
+
+        let mut expected: Vec<MusicButler> = 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::<NoLibrary, NoDatabase>::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());
+            .is_ok());
-        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);
+        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::<NoLibrary, NoDatabase>::new(None, None);
+
+        assert!(music_hoard.new_artist(&artist_id).is_ok());
+
+        let mut expected: Vec<Bandcamp> = 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::<NoLibrary, NoDatabase>::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.
+        // 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_err());
+        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_ok());
-        assert!(music_hoard.remove_qobuz_url(&artist_id, QOBUZ).is_err());
+        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::<NoLibrary, NoDatabase>::new(None, None);
+
+        assert!(music_hoard.new_artist(&artist_id).is_ok());
+
+        let mut expected: Option<Qobuz> = 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.