Finish with collection

2024-01-21 21:56:19 +01:00 · 2024-01-21 21:56:19 +01:00 · c4f7d841d5
commit c4f7d841d5
parent fd88edb275
4 changed files with 61 additions and 89 deletions
--- a/src/core/collection/artist.rs
+++ b/src/core/collection/artist.rs
@ -10,9 +10,7 @@ use uuid::Uuid;

 use super::album::Album;
 use super::merge::{Merge, MergeSorted};
-
-// FIXME: these imports are not acceptable
-use crate::core::musichoard::Error;
+use super::Error;

 /// An artist.
 #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
@ -123,9 +121,8 @@ impl Artist {
        match container {
            Some(current) => {
                if current != &url {
-                    return Err(Error::CollectionError(format!(
-                        "artist already has a different URL: {}",
-                        current
+                    return Err(Error::UrlError(format!(
+                        "artist already has a different URL: {current}"
                    )));
                }
            }
@ -289,22 +286,19 @@ impl MusicBrainz {
            .map(|u| u.ends_with("musicbrainz.org"))
            .unwrap_or(false)
        {
-            return Err(Self::invalid_url_error(url).into());
+            return Err(Self::invalid_url_error(url));
        }

        match url.path_segments().and_then(|mut ps| ps.nth(1)) {
            Some(segment) => Uuid::try_parse(segment)?,
-            None => return Err(Self::invalid_url_error(url).into()),
+            None => return Err(Self::invalid_url_error(url)),
        };

        Ok(MusicBrainz(url))
    }

-    fn invalid_url_error<S: Into<String>>(url: S) -> InvalidUrlError {
-        InvalidUrlError {
-            url_type: UrlType::MusicBrainz,
-            url: url.into(),
-        }
+    fn invalid_url_error<U: Display>(url: U) -> Error {
+        Error::UrlError(format!("invalid MusicBrainz URL: {url}"))
    }
 }

@ -349,7 +343,7 @@ impl MusicButler {
            .map(|u| u.ends_with("musicbutler.io"))
            .unwrap_or(false)
        {
-            return Err(Self::invalid_url_error(url).into());
+            return Err(Self::invalid_url_error(url));
        }

        Ok(MusicButler(url))
@ -359,11 +353,8 @@ impl MusicButler {
        self.0.as_str()
    }

-    fn invalid_url_error<S: Into<String>>(url: S) -> InvalidUrlError {
-        InvalidUrlError {
-            url_type: UrlType::MusicButler,
-            url: url.into(),
-        }
+    fn invalid_url_error<U: Display>(url: U) -> Error {
+        Error::UrlError(format!("invalid MusicButler URL: {url}"))
    }
 }

@ -395,7 +386,7 @@ impl Bandcamp {
            .map(|u| u.ends_with("bandcamp.com"))
            .unwrap_or(false)
        {
-            return Err(Self::invalid_url_error(url).into());
+            return Err(Self::invalid_url_error(url));
        }

        Ok(Bandcamp(url))
@ -405,11 +396,8 @@ impl Bandcamp {
        self.0.as_str()
    }

-    fn invalid_url_error<S: Into<String>>(url: S) -> InvalidUrlError {
-        InvalidUrlError {
-            url_type: UrlType::Bandcamp,
-            url: url.into(),
-        }
+    fn invalid_url_error<U: Display>(url: U) -> Error {
+        Error::UrlError(format!("invalid Bandcamp URL: {url}"))
    }
 }

@ -441,17 +429,14 @@ impl Qobuz {
            .map(|u| u.ends_with("qobuz.com"))
            .unwrap_or(false)
        {
-            return Err(Self::invalid_url_error(url).into());
+            return Err(Self::invalid_url_error(url));
        }

        Ok(Qobuz(url))
    }

-    fn invalid_url_error<S: Into<String>>(url: S) -> InvalidUrlError {
-        InvalidUrlError {
-            url_type: UrlType::Qobuz,
-            url: url.into(),
-        }
+    fn invalid_url_error<U: Display>(url: U) -> Error {
+        Error::UrlError(format!("invalid Qobuz URL: {url}"))
    }
 }

@ -475,28 +460,6 @@ impl Display for Qobuz {
    }
 }

-/// The different URL types supported by MusicHoard.
-#[derive(Debug)]
-enum UrlType {
-    MusicBrainz,
-    MusicButler,
-    Bandcamp,
-    Qobuz,
-}
-
-/// Invalid URL error.
-// FIXME: should not be public (or at least not in this form)
-pub struct InvalidUrlError {
-    url_type: UrlType,
-    url: String,
-}
-
-impl Display for InvalidUrlError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "invalid url of type {:?}: {}", self.url_type, self.url)
-    }
-}
-
 #[cfg(test)]
 mod tests {
    use super::*;
@ -522,11 +485,7 @@ mod tests {
        assert_eq!(actual_error.to_string(), expected_error.to_string());

        let url = "https://musicbrainz.org/artist".to_string();
-        let expected_error: Error = InvalidUrlError {
-            url_type: UrlType::MusicBrainz,
-            url: url.clone(),
-        }
-        .into();
+        let expected_error = Error::UrlError(format!("invalid MusicBrainz URL: {url}"));
        let actual_error = MusicBrainz::new(&url).unwrap_err();
        assert_eq!(actual_error, expected_error);
        assert_eq!(actual_error.to_string(), expected_error.to_string());
--- a/src/core/collection/mod.rs
+++ b/src/core/collection/mod.rs
@ -5,5 +5,34 @@ pub mod track;
 mod merge;
 pub use merge::Merge;

+use std::fmt::{self, Display};
+
 /// The [`Collection`] alias type for convenience.
 pub type Collection = Vec<artist::Artist>;
+
+/// Error type for the [`collection`] module.
+#[derive(Debug, PartialEq, Eq)]
+pub enum Error {
+    /// An error occurred when processing a URL.
+    UrlError(String),
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            Self::UrlError(ref s) => write!(f, "an error occurred when processing a URL: {s}"),
+        }
+    }
+}
+
+impl From<url::ParseError> for Error {
+    fn from(err: url::ParseError) -> Error {
+        Error::UrlError(err.to_string())
+    }
+}
+
+impl From<uuid::Error> for Error {
+    fn from(err: uuid::Error) -> Error {
+        Error::UrlError(err.to_string())
+    }
+}
--- a/src/core/musichoard/mod.rs
+++ b/src/core/musichoard/mod.rs
@ -4,8 +4,9 @@ use std::{collections::HashMap, mem};
 use paste::paste;

 use super::collection::{
+    self,
    album::{Album, AlbumId},
-    artist::{Artist, ArtistId, InvalidUrlError},
+    artist::{Artist, ArtistId},
    track::{Quality, Track, TrackId},
    Collection, Merge,
 };
@ -28,7 +29,7 @@ macro_rules! music_hoard_unique_url_dispatch {
                artist_id: ID,
                url: S,
            ) -> Result<(), Error> {
-                self.get_artist_mut_or_err(artist_id.as_ref())?.[<add_ $field _url>](url)
+                Ok(self.get_artist_mut_or_err(artist_id.as_ref())?.[<add_ $field _url>](url)?)
            }

            pub fn [<remove_ $field _url>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@ -36,7 +37,7 @@ macro_rules! music_hoard_unique_url_dispatch {
                artist_id: ID,
                url: S,
            ) -> Result<(), Error> {
-                self.get_artist_mut_or_err(artist_id.as_ref())?.[<remove_ $field _url>](url)
+                Ok(self.get_artist_mut_or_err(artist_id.as_ref())?.[<remove_ $field _url>](url)?)
            }

            pub fn [<set_ $field _url>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@ -44,7 +45,7 @@ macro_rules! music_hoard_unique_url_dispatch {
                artist_id: ID,
                url: S,
            ) -> Result<(), Error> {
-                self.get_artist_mut_or_err(artist_id.as_ref())?.[<set_ $field _url>](url)
+                Ok(self.get_artist_mut_or_err(artist_id.as_ref())?.[<set_ $field _url>](url)?)
            }

            pub fn [<clear_ $field _url>]<ID: AsRef<ArtistId>>(
@ -66,7 +67,7 @@ macro_rules! music_hoard_multi_url_dispatch {
                artist_id: ID,
                urls: Vec<S>,
            ) -> Result<(), Error> {
-                self.get_artist_mut_or_err(artist_id.as_ref())?.[<add_ $field _urls>](urls)
+                Ok(self.get_artist_mut_or_err(artist_id.as_ref())?.[<add_ $field _urls>](urls)?)
            }

            pub fn [<remove_ $field _urls>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@ -74,7 +75,7 @@ macro_rules! music_hoard_multi_url_dispatch {
                artist_id: ID,
                urls: Vec<S>,
            ) -> Result<(), Error> {
-                self.get_artist_mut_or_err(artist_id.as_ref())?.[<remove_ $field _urls>](urls)
+                Ok(self.get_artist_mut_or_err(artist_id.as_ref())?.[<remove_ $field _urls>](urls)?)
            }

            pub fn [<set_ $field _urls>]<ID: AsRef<ArtistId>, S: AsRef<str>>(
@ -82,7 +83,7 @@ macro_rules! music_hoard_multi_url_dispatch {
                artist_id: ID,
                urls: Vec<S>,
            ) -> Result<(), Error> {
-                self.get_artist_mut_or_err(artist_id.as_ref())?.[<set_ $field _urls>](urls)
+                Ok(self.get_artist_mut_or_err(artist_id.as_ref())?.[<set_ $field _urls>](urls)?)
            }

            pub fn [<clear_ $field _urls>]<ID: AsRef<ArtistId>>(
@ -379,10 +380,6 @@ pub enum Error {
    LibraryError(String),
    /// The [`MusicHoard`] failed to read/write from/to the database.
    DatabaseError(String),
-    /// The [`MusicHoard`] failed to parse a user-provided URL.
-    UrlParseError(String),
-    /// The user-provided URL is not valid.
-    InvalidUrlError(String),
 }

 impl Display for Error {
@ -393,12 +390,16 @@ impl Display for Error {
            Self::DatabaseError(ref s) => {
                write!(f, "failed to read/write from/to the database: {s}")
            }
-            Self::UrlParseError(ref s) => write!(f, "failed to parse a user-provided URL: {s}"),
-            Self::InvalidUrlError(ref s) => write!(f, "user-provided URL is invalid: {s}"),
        }
    }
 }

+impl From<collection::Error> for Error {
+    fn from(err: collection::Error) -> Self {
+        Error::CollectionError(err.to_string())
+    }
+}
+
 impl From<library::Error> for Error {
    fn from(err: library::Error) -> Error {
        Error::LibraryError(err.to_string())
@ -417,24 +418,6 @@ impl From<database::SaveError> for Error {
    }
 }

-impl From<url::ParseError> for Error {
-    fn from(err: url::ParseError) -> Error {
-        Error::UrlParseError(err.to_string())
-    }
-}
-
-impl From<uuid::Error> for Error {
-    fn from(err: uuid::Error) -> Error {
-        Error::UrlParseError(err.to_string())
-    }
-}
-
-impl From<InvalidUrlError> for Error {
-    fn from(err: InvalidUrlError) -> Error {
-        Error::InvalidUrlError(err.to_string())
-    }
-}
-
 #[cfg(test)]
 pub mod testmod;

--- a/src/core/musichoard/tests.rs
+++ b/src/core/musichoard/tests.rs
@ -22,6 +22,7 @@ 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";

+// FIXME: all tests with URLs should go to collection::artist
 #[test]
 fn urls() {
    assert!(MusicBrainz::new(MUSICBRAINZ).is_ok());