Code rearrangements

src/core/collection/album.rs

@@ -2,17 +2,8 @@ use core::mem;
 
 use serde::{Deserialize, Serialize};
 
-use super::track::Track;
-
-// FIXME: check direction of import.
 use super::merge::{Merge, MergeSorted};
-
-/// The album identifier.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, PartialOrd, Ord, Eq, Hash)]
-pub struct AlbumId {
-    pub year: u32,
-    pub title: String,
-}
+use super::track::Track;
 
 /// An album is a collection of tracks that were released together.
 #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
@@ -21,6 +12,13 @@ pub struct Album {
     pub tracks: Vec<Track>,
 }
 
+/// The album identifier.
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, PartialOrd, Ord, Eq, Hash)]
+pub struct AlbumId {
+    pub year: u32,
+    pub title: String,
+}
+
 impl PartialOrd for Album {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         Some(self.cmp(other))

src/core/collection/artist.rs

@@ -9,41 +9,272 @@ use url::Url;
 use uuid::Uuid;
 
 use super::album::Album;
-
-// FIXME: check direction of import.
 use super::merge::{Merge, MergeSorted};
 
 // FIXME: these imports are not acceptable
 use crate::core::musichoard::Error;
 
+/// An artist.
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+pub struct Artist {
+    pub id: ArtistId,
+    pub sort: Option<ArtistId>,
+    pub properties: ArtistProperties,
+    pub albums: Vec<Album>,
+}
+
+/// The artist identifier.
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct ArtistId {
+    pub name: String,
+}
+
+/// The artist properties.
+#[derive(Clone, Debug, Default, Deserialize, Serialize, PartialEq, Eq)]
+pub struct ArtistProperties {
+    pub musicbrainz: Option<MusicBrainz>,
+    pub musicbutler: Vec<MusicButler>,
+    pub bandcamp: Vec<Bandcamp>,
+    pub qobuz: Option<Qobuz>,
+}
+
+macro_rules! artist_unique_url_dispatch {
+    ($field:ident) => {
+        paste! {
+            pub fn [<add_ $field _url>]<S: AsRef<str>>(&mut self, url: S) -> Result<(), Error> {
+                Self::add_unique_url(&mut self.properties.$field, url)
+            }
+
+            pub fn [<remove_ $field _url>]<S: AsRef<str>>(&mut self, url: S) -> Result<(), Error> {
+                Self::remove_unique_url(&mut self.properties.$field, url)
+            }
+
+            pub fn [<set_ $field _url>]<S: AsRef<str>>(&mut self, url: S) -> Result<(), Error> {
+                Self::set_unique_url(&mut self.properties.$field, url)
+            }
+
+            pub fn [<clear_ $field _url>](&mut self) {
+                Self::clear_unique_url(&mut self.properties.$field);
+            }
+        }
+    };
+}
+
+macro_rules! artist_multi_url_dispatch {
+    ($field:ident) => {
+        paste! {
+            pub fn [<add_ $field _urls>]<S: AsRef<str>>(
+                &mut self,
+                urls: Vec<S>,
+            ) -> Result<(), Error> {
+                Self::add_multi_urls(&mut self.properties.$field, urls)
+            }
+
+            pub fn [<remove_ $field _urls>]<S: AsRef<str>>(
+                &mut self,
+                urls: Vec<S>,
+            ) -> Result<(), Error> {
+                Self::remove_multi_urls(&mut self.properties.$field, urls)
+            }
+
+            pub fn [<set_ $field _urls>]<S: AsRef<str>>(
+                &mut self,
+                urls: Vec<S>,
+            ) -> Result<(), Error> {
+                Self::set_multi_urls(&mut self.properties.$field, urls)
+            }
+
+            pub fn [<clear_ $field _urls>](&mut self) {
+                Self::clear_multi_urls(&mut self.properties.$field);
+            }
+        }
+    };
+}
+
+impl Artist {
+    /// Create new [`Artist`] with the given [`ArtistId`].
+    pub fn new<ID: Into<ArtistId>>(id: ID) -> Self {
+        Artist {
+            id: id.into(),
+            sort: None,
+            properties: ArtistProperties::default(),
+            albums: vec![],
+        }
+    }
+
+    fn get_sort_key(&self) -> &ArtistId {
+        self.sort.as_ref().unwrap_or(&self.id)
+    }
+
+    pub fn set_sort_key<SORT: Into<ArtistId>>(&mut self, sort: SORT) {
+        self.sort = Some(sort.into());
+    }
+
+    pub fn clear_sort_key(&mut self) {
+        _ = self.sort.take();
+    }
+
+    fn add_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq + Display>(
+        container: &mut Option<T>,
+        url: S,
+    ) -> Result<(), Error> {
+        let url: T = url.as_ref().try_into()?;
+
+        match container {
+            Some(current) => {
+                if current != &url {
+                    return Err(Error::CollectionError(format!(
+                        "artist already has a different URL: {}",
+                        current
+                    )));
+                }
+            }
+            None => {
+                _ = container.insert(url);
+            }
+        }
+
+        Ok(())
+    }
+
+    fn remove_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
+        container: &mut Option<T>,
+        url: S,
+    ) -> Result<(), Error> {
+        let url: T = url.as_ref().try_into()?;
+
+        if container == &Some(url) {
+            _ = container.take();
+        }
+
+        Ok(())
+    }
+
+    fn set_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error>>(
+        container: &mut Option<T>,
+        url: S,
+    ) -> Result<(), Error> {
+        _ = container.insert(url.as_ref().try_into()?);
+        Ok(())
+    }
+
+    fn clear_unique_url<T>(container: &mut Option<T>) {
+        _ = container.take();
+    }
+
+    fn add_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
+        container: &mut Vec<T>,
+        urls: Vec<S>,
+    ) -> Result<(), Error> {
+        let mut new_urls = urls
+            .iter()
+            .map(|url| url.as_ref().try_into())
+            .filter(|res| {
+                res.as_ref()
+                    .map(|url| !container.contains(url))
+                    .unwrap_or(true) // Propagate errors.
+            })
+            .collect::<Result<Vec<T>, Error>>()?;
+
+        container.append(&mut new_urls);
+        Ok(())
+    }
+
+    fn remove_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
+        container: &mut Vec<T>,
+        urls: Vec<S>,
+    ) -> Result<(), Error> {
+        let urls = urls
+            .iter()
+            .map(|url| url.as_ref().try_into())
+            .collect::<Result<Vec<T>, Error>>()?;
+
+        container.retain(|url| !urls.contains(url));
+        Ok(())
+    }
+
+    fn set_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error>>(
+        container: &mut Vec<T>,
+        urls: Vec<S>,
+    ) -> Result<(), Error> {
+        let mut urls = urls
+            .iter()
+            .map(|url| url.as_ref().try_into())
+            .collect::<Result<Vec<T>, Error>>()?;
+
+        container.clear();
+        container.append(&mut urls);
+        Ok(())
+    }
+
+    fn clear_multi_urls<T>(container: &mut Vec<T>) {
+        container.clear();
+    }
+
+    artist_unique_url_dispatch!(musicbrainz);
+
+    artist_multi_url_dispatch!(musicbutler);
+
+    artist_multi_url_dispatch!(bandcamp);
+
+    artist_unique_url_dispatch!(qobuz);
+}
+
+impl PartialOrd for Artist {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for Artist {
+    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
+        self.get_sort_key().cmp(other.get_sort_key())
+    }
+}
+
+impl Merge for Artist {
+    fn merge_in_place(&mut self, other: Self) {
+        assert_eq!(self.id, other.id);
+        self.sort = self.sort.take().or(other.sort);
+        self.properties.merge_in_place(other.properties);
+        let albums = mem::take(&mut self.albums);
+        self.albums = MergeSorted::new(albums.into_iter(), other.albums.into_iter()).collect();
+    }
+}
+
+impl AsRef<ArtistId> for ArtistId {
+    fn as_ref(&self) -> &ArtistId {
+        self
+    }
+}
+
+impl ArtistId {
+    pub fn new<S: Into<String>>(name: S) -> ArtistId {
+        ArtistId { name: name.into() }
+    }
+}
+
+impl Display for ArtistId {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "{}", self.name)
+    }
+}
+
+impl Merge for ArtistProperties {
+    fn merge_in_place(&mut self, other: Self) {
+        self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz);
+        Self::merge_vecs(&mut self.musicbutler, other.musicbutler);
+        Self::merge_vecs(&mut self.bandcamp, other.bandcamp);
+        self.qobuz = self.qobuz.take().or(other.qobuz);
+    }
+}
+
 /// An object with the [`IMbid`] trait contains a [MusicBrainz
 /// Identifier](https://musicbrainz.org/doc/MusicBrainz_Identifier) (MBID).
 pub trait IMbid {
     fn mbid(&self) -> &str;
 }
 
-/// 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)
-    }
-}
-
 /// MusicBrainz reference.
 #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)]
 pub struct MusicBrainz(Url);
@@ -244,249 +475,25 @@ impl Display for Qobuz {
     }
 }
 
-/// The artist identifier.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct ArtistId {
-    pub name: String,
+/// The different URL types supported by MusicHoard.
+#[derive(Debug)]
+enum UrlType {
+    MusicBrainz,
+    MusicButler,
+    Bandcamp,
+    Qobuz,
 }
 
-impl AsRef<ArtistId> for ArtistId {
-    fn as_ref(&self) -> &ArtistId {
-        self
-    }
+/// Invalid URL error.
+// FIXME: should not be public (or at least not in this form)
+pub struct InvalidUrlError {
+    url_type: UrlType,
+    url: String,
 }
 
-impl ArtistId {
-    pub fn new<S: Into<String>>(name: S) -> ArtistId {
-        ArtistId { name: name.into() }
-    }
-}
-
-impl Display for ArtistId {
+impl Display for InvalidUrlError {
     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 {
-    pub musicbrainz: Option<MusicBrainz>,
-    pub musicbutler: Vec<MusicButler>,
-    pub bandcamp: Vec<Bandcamp>,
-    pub qobuz: Option<Qobuz>,
-}
-
-impl Merge for ArtistProperties {
-    fn merge_in_place(&mut self, other: Self) {
-        self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz);
-        Self::merge_vecs(&mut self.musicbutler, other.musicbutler);
-        Self::merge_vecs(&mut self.bandcamp, other.bandcamp);
-        self.qobuz = self.qobuz.take().or(other.qobuz);
-    }
-}
-
-/// An artist.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
-pub struct Artist {
-    pub id: ArtistId,
-    pub sort: Option<ArtistId>,
-    pub properties: ArtistProperties,
-    pub albums: Vec<Album>,
-}
-
-macro_rules! artist_unique_url_dispatch {
-    ($field:ident) => {
-        paste! {
-            pub fn [<add_ $field _url>]<S: AsRef<str>>(&mut self, url: S) -> Result<(), Error> {
-                Self::add_unique_url(&mut self.properties.$field, url)
-            }
-
-            pub fn [<remove_ $field _url>]<S: AsRef<str>>(&mut self, url: S) -> Result<(), Error> {
-                Self::remove_unique_url(&mut self.properties.$field, url)
-            }
-
-            pub fn [<set_ $field _url>]<S: AsRef<str>>(&mut self, url: S) -> Result<(), Error> {
-                Self::set_unique_url(&mut self.properties.$field, url)
-            }
-
-            pub fn [<clear_ $field _url>](&mut self) {
-                Self::clear_unique_url(&mut self.properties.$field);
-            }
-        }
-    };
-}
-
-macro_rules! artist_multi_url_dispatch {
-    ($field:ident) => {
-        paste! {
-            pub fn [<add_ $field _urls>]<S: AsRef<str>>(&mut self, urls: Vec<S>) -> Result<(), Error> {
-                Self::add_multi_urls(&mut self.properties.$field, urls)
-            }
-
-            pub fn [<remove_ $field _urls>]<S: AsRef<str>>(&mut self, urls: Vec<S>) -> Result<(), Error> {
-                Self::remove_multi_urls(&mut self.properties.$field, urls)
-            }
-
-            pub fn [<set_ $field _urls>]<S: AsRef<str>>(&mut self, urls: Vec<S>) -> Result<(), Error> {
-                Self::set_multi_urls(&mut self.properties.$field, urls)
-            }
-
-            pub fn [<clear_ $field _urls>](&mut self) {
-                Self::clear_multi_urls(&mut self.properties.$field);
-            }
-        }
-    };
-}
-
-impl Artist {
-    /// Create new [`Artist`] with the given [`ArtistId`].
-    pub fn new<ID: Into<ArtistId>>(id: ID) -> Self {
-        Artist {
-            id: id.into(),
-            sort: None,
-            properties: ArtistProperties::default(),
-            albums: vec![],
-        }
-    }
-
-    fn get_sort_key(&self) -> &ArtistId {
-        self.sort.as_ref().unwrap_or(&self.id)
-    }
-
-    pub fn set_sort_key<SORT: Into<ArtistId>>(&mut self, sort: SORT) {
-        self.sort = Some(sort.into());
-    }
-
-    pub fn clear_sort_key(&mut self) {
-        _ = self.sort.take();
-    }
-
-    fn add_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq + Display>(
-        container: &mut Option<T>,
-        url: S,
-    ) -> Result<(), Error> {
-        let url: T = url.as_ref().try_into()?;
-
-        match container {
-            Some(current) => {
-                if current != &url {
-                    return Err(Error::CollectionError(format!(
-                        "artist already has a different URL: {}",
-                        current
-                    )));
-                }
-            }
-            None => {
-                _ = container.insert(url);
-            }
-        }
-
-        Ok(())
-    }
-
-    fn remove_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
-        container: &mut Option<T>,
-        url: S,
-    ) -> Result<(), Error> {
-        let url: T = url.as_ref().try_into()?;
-
-        if container == &Some(url) {
-            _ = container.take();
-        }
-
-        Ok(())
-    }
-
-    fn set_unique_url<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error>>(
-        container: &mut Option<T>,
-        url: S,
-    ) -> Result<(), Error> {
-        _ = container.insert(url.as_ref().try_into()?);
-        Ok(())
-    }
-
-    fn clear_unique_url<T>(container: &mut Option<T>) {
-        _ = container.take();
-    }
-
-    fn add_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
-        container: &mut Vec<T>,
-        urls: Vec<S>,
-    ) -> Result<(), Error> {
-        let mut new_urls = urls
-            .iter()
-            .map(|url| url.as_ref().try_into())
-            .filter(|res| {
-                res.as_ref()
-                    .map(|url| !container.contains(url))
-                    .unwrap_or(true) // Propagate errors.
-            })
-            .collect::<Result<Vec<T>, Error>>()?;
-
-        container.append(&mut new_urls);
-        Ok(())
-    }
-
-    fn remove_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error> + Eq>(
-        container: &mut Vec<T>,
-        urls: Vec<S>,
-    ) -> Result<(), Error> {
-        let urls = urls
-            .iter()
-            .map(|url| url.as_ref().try_into())
-            .collect::<Result<Vec<T>, Error>>()?;
-
-        container.retain(|url| !urls.contains(url));
-        Ok(())
-    }
-
-    fn set_multi_urls<S: AsRef<str>, T: for<'a> TryFrom<&'a str, Error = Error>>(
-        container: &mut Vec<T>,
-        urls: Vec<S>,
-    ) -> Result<(), Error> {
-        let mut urls = urls
-            .iter()
-            .map(|url| url.as_ref().try_into())
-            .collect::<Result<Vec<T>, Error>>()?;
-
-        container.clear();
-        container.append(&mut urls);
-        Ok(())
-    }
-
-    fn clear_multi_urls<T>(container: &mut Vec<T>) {
-        container.clear();
-    }
-
-    artist_unique_url_dispatch!(musicbrainz);
-
-    artist_multi_url_dispatch!(musicbutler);
-
-    artist_multi_url_dispatch!(bandcamp);
-
-    artist_unique_url_dispatch!(qobuz);
-}
-
-impl PartialOrd for Artist {
-    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl Ord for Artist {
-    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
-        self.get_sort_key().cmp(other.get_sort_key())
-    }
-}
-
-impl Merge for Artist {
-    fn merge_in_place(&mut self, other: Self) {
-        assert_eq!(self.id, other.id);
-        self.sort = self.sort.take().or(other.sort);
-        self.properties.merge_in_place(other.properties);
-        let albums = mem::take(&mut self.albums);
-        self.albums = MergeSorted::new(albums.into_iter(), other.albums.into_iter()).collect();
+        write!(f, "invalid url of type {:?}: {}", self.url_type, self.url)
     }
 }
 

src/core/collection/merge.rs

@@ -1,5 +1,7 @@
 use std::{cmp::Ordering, iter::Peekable};
 
+/// A trait for merging two objects. The merge is asymmetric with the left argument considered to be
+/// the primary whose properties are to be kept in case of collisions.
 pub trait Merge {
     fn merge_in_place(&mut self, other: Self);
 

src/core/collection/mod.rs

@@ -5,5 +5,5 @@ pub mod track;
 mod merge;
 pub use merge::Merge;
 
-/// The collection alias type for convenience.
+/// The [`Collection`] alias type for convenience.
 pub type Collection = Vec<artist::Artist>;

src/core/collection/track.rs

@@ -1,20 +1,13 @@
 use serde::{Deserialize, Serialize};
 
-// FIXME: check direction of import.
 use super::merge::Merge;
 
-/// The track file format.
-#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq, Hash)]
-pub enum Format {
-    Flac,
-    Mp3,
-}
-
-/// The track quality. Combines format and bitrate information.
-#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)]
-pub struct Quality {
-    pub format: Format,
-    pub bitrate: u32,
+/// A single track on an album.
+#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+pub struct Track {
+    pub id: TrackId,
+    pub artist: Vec<String>,
+    pub quality: Quality,
 }
 
 /// The track identifier.
@@ -24,12 +17,18 @@ pub struct TrackId {
     pub title: String,
 }
 
-/// A single track on an album.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
-pub struct Track {
-    pub id: TrackId,
-    pub artist: Vec<String>,
-    pub quality: Quality,
+/// The track quality. Combines format and bitrate information.
+#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)]
+pub struct Quality {
+    pub format: Format,
+    pub bitrate: u32,
+}
+
+/// The track file format.
+#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq, Hash)]
+pub enum Format {
+    Flac,
+    Mp3,
 }
 
 impl PartialOrd for Track {