For the database serde implementation use Mbid rather than MbRef (#199)

Closes #198

Reviewed-on: #199

examples/musicbrainz_api/lookup_artist.rs

@@ -1,11 +1,11 @@
 #![allow(non_snake_case)]
 
 use musichoard::{
+    collection::musicbrainz::Mbid,
     external::musicbrainz::{
         api::{lookup::LookupArtistRequest, MusicBrainzClient},
         http::MusicBrainzHttp,
     },
-    interface::musicbrainz::Mbid,
 };
 use structopt::StructOpt;
 use uuid::Uuid;

examples/musicbrainz_api/search_release_group.rs

@@ -3,11 +3,11 @@
 use std::{num::ParseIntError, str::FromStr};
 
 use musichoard::{
-    collection::album::AlbumDate,
+    collection::{album::AlbumDate, musicbrainz::Mbid},
     external::musicbrainz::{
-        api::search::SearchReleaseGroupRequest, api::MusicBrainzClient, http::MusicBrainzHttp,
+        api::{search::SearchReleaseGroupRequest, MusicBrainzClient},
+        http::MusicBrainzHttp,
     },
-    interface::musicbrainz::Mbid,
 };
 use structopt::StructOpt;
 use uuid::Uuid;

src/core/collection/mod.rs

@@ -16,6 +16,8 @@ pub type Collection = Vec<artist::Artist>;
 /// Error type for the [`collection`] module.
 #[derive(Debug, PartialEq, Eq)]
 pub enum Error {
+    /// An error occurred when processing an MBID.
+    MbidError(String),
     /// An error occurred when processing a URL.
     UrlError(String),
 }
@@ -23,6 +25,7 @@ pub enum Error {
 impl Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
+            Self::MbidError(ref s) => write!(f, "an error occurred when processing an MBID: {s}"),
             Self::UrlError(ref s) => write!(f, "an error occurred when processing a URL: {s}"),
         }
     }
@@ -36,6 +39,6 @@ impl From<url::ParseError> for Error {
 
 impl From<uuid::Error> for Error {
     fn from(err: uuid::Error) -> Error {
-        Error::UrlError(err.to_string())
+        Error::MbidError(err.to_string())
     }
 }

src/core/collection/musicbrainz.rs

@@ -3,10 +3,41 @@ use std::fmt::{Debug, Display};
 use url::Url;
 use uuid::Uuid;
 
-use crate::{core::collection::Error, interface::musicbrainz::Mbid};
+use crate::core::collection::Error;
 
 const MB_DOMAIN: &str = "musicbrainz.org";
 
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct Mbid(Uuid);
+
+impl Mbid {
+    pub fn uuid(&self) -> &Uuid {
+        &self.0
+    }
+}
+
+impl From<Uuid> for Mbid {
+    fn from(value: Uuid) -> Self {
+        Mbid(value)
+    }
+}
+
+macro_rules! try_from_impl_for_mbid {
+    ($from:ty) => {
+        impl TryFrom<$from> for Mbid {
+            type Error = Error;
+
+            fn try_from(value: $from) -> Result<Self, Self::Error> {
+                Ok(Uuid::parse_str(value.as_ref())?.into())
+            }
+        }
+    };
+}
+
+try_from_impl_for_mbid!(&str);
+try_from_impl_for_mbid!(&String);
+try_from_impl_for_mbid!(String);
+
 #[derive(Clone, Debug, PartialEq, Eq)]
 struct MusicBrainzRef {
     mbid: Mbid,

src/core/interface/database/mod.rs

@@ -59,7 +59,9 @@ impl From<std::io::Error> for LoadError {
 impl From<collection::Error> for LoadError {
     fn from(err: collection::Error) -> Self {
         match err {
-            collection::Error::UrlError(e) => LoadError::SerDeError(e),
+            collection::Error::UrlError(e) | collection::Error::MbidError(e) => {
+                LoadError::SerDeError(e)
+            }
         }
     }
 }

src/core/interface/mod.rs

@@ -1,3 +1,2 @@
 pub mod database;
 pub mod library;
-pub mod musicbrainz;

src/core/interface/musicbrainz/mod.rs

@@ -1,56 +0,0 @@
-use std::fmt;
-
-use uuid::{self, Uuid};
-
-#[derive(Clone, Debug, PartialEq, Eq)]
-pub struct Mbid(Uuid);
-
-impl Mbid {
-    pub fn uuid(&self) -> &Uuid {
-        &self.0
-    }
-}
-
-impl From<Uuid> for Mbid {
-    fn from(value: Uuid) -> Self {
-        Mbid(value)
-    }
-}
-
-#[derive(Debug)]
-pub struct MbidError(String);
-
-impl fmt::Display for MbidError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "failed to parse a MBID: {}", self.0)
-    }
-}
-
-impl From<uuid::Error> for MbidError {
-    fn from(value: uuid::Error) -> Self {
-        MbidError(value.to_string())
-    }
-}
-
-macro_rules! try_from_impl_for_mbid {
-    ($from:ty) => {
-        impl TryFrom<$from> for Mbid {
-            type Error = MbidError;
-
-            fn try_from(value: $from) -> Result<Self, Self::Error> {
-                Ok(Uuid::parse_str(value.as_ref())?.into())
-            }
-        }
-    };
-}
-
-try_from_impl_for_mbid!(&str);
-try_from_impl_for_mbid!(&String);
-try_from_impl_for_mbid!(String);
-
-#[test]
-fn errors() {
-    let mbid_err: MbidError = TryInto::<Mbid>::try_into("i-am-not-a-uuid").unwrap_err();
-    assert!(!mbid_err.to_string().is_empty());
-    assert!(!format!("{mbid_err:?}").is_empty());
-}

src/external/database/serde/deserialize.rs

@@ -3,10 +3,10 @@ use std::{collections::HashMap, fmt};
 use serde::{de::Visitor, Deserialize, Deserializer};
 
 use crate::{
+    collection::musicbrainz::Mbid,
     core::collection::{
         album::{Album, AlbumDate, AlbumId, AlbumSeq},
         artist::{Artist, ArtistId},
-        musicbrainz::{MbAlbumRef, MbArtistRef},
         Collection, Error as CollectionError,
     },
     external::database::serde::common::{SerdeAlbumPrimaryType, SerdeAlbumSecondaryType},
@@ -31,7 +31,7 @@ impl From<DeserializeDatabase> for Collection {
 pub struct DeserializeArtist {
     name: String,
     sort: Option<String>,
-    musicbrainz: Option<DeserializeMbArtistRef>,
+    musicbrainz: Option<DeserializeMbid>,
     properties: HashMap<String, Vec<String>>,
     albums: Vec<DeserializeAlbum>,
 }
@@ -40,24 +40,24 @@ pub struct DeserializeArtist {
 pub struct DeserializeAlbum {
     title: String,
     seq: u8,
-    musicbrainz: Option<DeserializeMbAlbumRef>,
+    musicbrainz: Option<DeserializeMbid>,
     primary_type: Option<SerdeAlbumPrimaryType>,
     secondary_types: Vec<SerdeAlbumSecondaryType>,
 }
 
 #[derive(Clone, Debug)]
-pub struct DeserializeMbArtistRef(MbArtistRef);
+pub struct DeserializeMbid(Mbid);
 
-impl From<DeserializeMbArtistRef> for MbArtistRef {
+impl From<DeserializeMbid> for Mbid {
-    fn from(value: DeserializeMbArtistRef) -> Self {
+    fn from(value: DeserializeMbid) -> Self {
         value.0
     }
 }
 
-struct DeserializeMbArtistRefVisitor;
+struct DeserializeMbidVisitor;
 
-impl<'de> Visitor<'de> for DeserializeMbArtistRefVisitor {
+impl<'de> Visitor<'de> for DeserializeMbidVisitor {
-    type Value = DeserializeMbArtistRef;
+    type Value = DeserializeMbid;
 
     fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
         formatter.write_str("a valid MusicBrainz identifier")
@@ -67,55 +67,19 @@ impl<'de> Visitor<'de> for DeserializeMbArtistRefVisitor {
     where
         E: serde::de::Error,
     {
-        Ok(DeserializeMbArtistRef(
+        Ok(DeserializeMbid(
-            MbArtistRef::from_uuid_str(v).map_err(|e: CollectionError| E::custom(e.to_string()))?,
+            v.try_into()
+                .map_err(|e: CollectionError| E::custom(e.to_string()))?,
         ))
     }
 }
 
-impl<'de> Deserialize<'de> for DeserializeMbArtistRef {
+impl<'de> Deserialize<'de> for DeserializeMbid {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
         D: Deserializer<'de>,
     {
-        deserializer.deserialize_str(DeserializeMbArtistRefVisitor)
+        deserializer.deserialize_str(DeserializeMbidVisitor)
-    }
-}
-
-#[derive(Clone, Debug)]
-pub struct DeserializeMbAlbumRef(MbAlbumRef);
-
-impl From<DeserializeMbAlbumRef> for MbAlbumRef {
-    fn from(value: DeserializeMbAlbumRef) -> Self {
-        value.0
-    }
-}
-
-struct DeserializeMbAlbumRefVisitor;
-
-impl<'de> Visitor<'de> for DeserializeMbAlbumRefVisitor {
-    type Value = DeserializeMbAlbumRef;
-
-    fn expecting(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
-        formatter.write_str("a valid MusicBrainz identifier")
-    }
-
-    fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
-    where
-        E: serde::de::Error,
-    {
-        Ok(DeserializeMbAlbumRef(
-            MbAlbumRef::from_uuid_str(v).map_err(|e: CollectionError| E::custom(e.to_string()))?,
-        ))
-    }
-}
-
-impl<'de> Deserialize<'de> for DeserializeMbAlbumRef {
-    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-    where
-        D: Deserializer<'de>,
-    {
-        deserializer.deserialize_str(DeserializeMbAlbumRefVisitor)
     }
 }
 
@@ -124,7 +88,7 @@ impl From<DeserializeArtist> for Artist {
         Artist {
             id: ArtistId::new(artist.name),
             sort: artist.sort.map(ArtistId::new),
-            musicbrainz: artist.musicbrainz.map(Into::into),
+            musicbrainz: artist.musicbrainz.map(Into::<Mbid>::into).map(Into::into),
             properties: artist.properties,
             albums: artist.albums.into_iter().map(Into::into).collect(),
         }
@@ -137,7 +101,7 @@ impl From<DeserializeAlbum> for Album {
             id: AlbumId { title: album.title },
             date: AlbumDate::default(),
             seq: AlbumSeq(album.seq),
-            musicbrainz: album.musicbrainz.map(Into::into),
+            musicbrainz: album.musicbrainz.map(Into::<Mbid>::into).map(Into::into),
             primary_type: album.primary_type.map(Into::into),
             secondary_types: album.secondary_types.into_iter().map(Into::into).collect(),
             tracks: vec![],
@@ -150,35 +114,17 @@ mod tests {
     use super::*;
 
     #[test]
-    fn deserialize_mb_artist_ref() {
+    fn deserialize_mbid() {
         let mbid = "\"d368baa8-21ca-4759-9731-0b2753071ad8\"";
-        let mbref: DeserializeMbArtistRef = serde_json::from_str(mbid).unwrap();
+        let mbid: DeserializeMbid = serde_json::from_str(mbid).unwrap();
-        let mbref: MbArtistRef = mbref.into();
+        let mbid: Mbid = mbid.into();
         assert_eq!(
-            mbref,
+            mbid,
-            MbArtistRef::from_uuid_str("d368baa8-21ca-4759-9731-0b2753071ad8").unwrap()
+            "d368baa8-21ca-4759-9731-0b2753071ad8".try_into().unwrap()
         );
 
         let mbid = "null";
-        let result: Result<DeserializeMbArtistRef, _> = serde_json::from_str(mbid);
+        let result: Result<DeserializeMbid, _> = serde_json::from_str(mbid);
-        assert!(result
-            .unwrap_err()
-            .to_string()
-            .contains("a valid MusicBrainz identifier"));
-    }
-
-    #[test]
-    fn deserialize_mb_album_ref() {
-        let mbid = "\"d368baa8-21ca-4759-9731-0b2753071ad8\"";
-        let mbref: DeserializeMbAlbumRef = serde_json::from_str(mbid).unwrap();
-        let mbref: MbAlbumRef = mbref.into();
-        assert_eq!(
-            mbref,
-            MbAlbumRef::from_uuid_str("d368baa8-21ca-4759-9731-0b2753071ad8").unwrap()
-        );
-
-        let mbid = "null";
-        let result: Result<DeserializeMbAlbumRef, _> = serde_json::from_str(mbid);
         assert!(result
             .unwrap_err()
             .to_string()

src/external/database/serde/serialize.rs

@@ -3,7 +3,7 @@ use std::collections::BTreeMap;
 use serde::Serialize;
 
 use crate::{
-    collection::musicbrainz::{MbAlbumRef, MbArtistRef},
+    collection::musicbrainz::Mbid,
     core::collection::{album::Album, artist::Artist, musicbrainz::IMusicBrainzRef, Collection},
     external::database::serde::common::{SerdeAlbumPrimaryType, SerdeAlbumSecondaryType},
 };
@@ -23,7 +23,7 @@ impl<'a> From<&'a Collection> for SerializeDatabase<'a> {
 pub struct SerializeArtist<'a> {
     name: &'a str,
     sort: Option<&'a str>,
-    musicbrainz: Option<SerializeMbArtistRef<'a>>,
+    musicbrainz: Option<SerializeMbid<'a>>,
     properties: BTreeMap<&'a str, &'a Vec<String>>,
     albums: Vec<SerializeAlbum<'a>>,
 }
@@ -32,32 +32,20 @@ pub struct SerializeArtist<'a> {
 pub struct SerializeAlbum<'a> {
     title: &'a str,
     seq: u8,
-    musicbrainz: Option<SerializeMbAlbumRef<'a>>,
+    musicbrainz: Option<SerializeMbid<'a>>,
     primary_type: Option<SerdeAlbumPrimaryType>,
     secondary_types: Vec<SerdeAlbumSecondaryType>,
 }
 
 #[derive(Clone, Debug)]
-pub struct SerializeMbArtistRef<'a>(&'a MbArtistRef);
+pub struct SerializeMbid<'a>(&'a Mbid);
 
-impl<'a> Serialize for SerializeMbArtistRef<'a> {
+impl<'a> Serialize for SerializeMbid<'a> {
     fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
-        serializer.serialize_str(&self.0.mbid().uuid().as_hyphenated().to_string())
+        serializer.serialize_str(&self.0.uuid().as_hyphenated().to_string())
-    }
-}
-
-#[derive(Clone, Debug)]
-pub struct SerializeMbAlbumRef<'a>(&'a MbAlbumRef);
-
-impl<'a> Serialize for SerializeMbAlbumRef<'a> {
-    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-    where
-        S: serde::Serializer,
-    {
-        serializer.serialize_str(&self.0.mbid().uuid().as_hyphenated().to_string())
     }
 }
 
@@ -66,7 +54,10 @@ impl<'a> From<&'a Artist> for SerializeArtist<'a> {
         SerializeArtist {
             name: &artist.id.name,
             sort: artist.sort.as_ref().map(|id| id.name.as_ref()),
-            musicbrainz: artist.musicbrainz.as_ref().map(SerializeMbArtistRef),
+            musicbrainz: artist
+                .musicbrainz
+                .as_ref()
+                .map(|mbref| SerializeMbid(mbref.mbid())),
             properties: artist
                 .properties
                 .iter()
@@ -82,7 +73,10 @@ impl<'a> From<&'a Album> for SerializeAlbum<'a> {
         SerializeAlbum {
             title: &album.id.title,
             seq: album.seq.0,
-            musicbrainz: album.musicbrainz.as_ref().map(SerializeMbAlbumRef),
+            musicbrainz: album
+                .musicbrainz
+                .as_ref()
+                .map(|mbref| SerializeMbid(mbref.mbid())),
             primary_type: album.primary_type.map(Into::into),
             secondary_types: album
                 .secondary_types

src/external/musicbrainz/api/lookup.rs

@@ -2,7 +2,10 @@ use serde::Deserialize;
 use url::form_urlencoded;
 
 use crate::{
-    collection::album::{AlbumDate, AlbumPrimaryType, AlbumSecondaryType},
+    collection::{
+        album::{AlbumDate, AlbumPrimaryType, AlbumSecondaryType},
+        musicbrainz::Mbid,
+    },
     external::musicbrainz::{
         api::{
             Error, MusicBrainzClient, SerdeAlbumDate, SerdeAlbumPrimaryType,
@@ -10,7 +13,6 @@ use crate::{
         },
         IMusicBrainzHttp,
     },
-    interface::musicbrainz::Mbid,
 };
 
 impl<Http: IMusicBrainzHttp> MusicBrainzClient<Http> {

src/external/musicbrainz/api/mod.rs

@@ -3,9 +3,12 @@ use std::{fmt, num};
 use serde::{de::Visitor, Deserialize, Deserializer};
 
 use crate::{
-    collection::album::{AlbumDate, AlbumPrimaryType, AlbumSecondaryType},
+    collection::{
+        album::{AlbumDate, AlbumPrimaryType, AlbumSecondaryType},
+        musicbrainz::Mbid,
+        Error as CollectionError,
+    },
     external::musicbrainz::HttpError,
-    interface::musicbrainz::{Mbid, MbidError},
 };
 
 pub mod lookup;
@@ -92,7 +95,7 @@ impl<'de> Visitor<'de> for SerdeMbidVisitor {
     {
         Ok(SerdeMbid(
             v.try_into()
-                .map_err(|e: MbidError| E::custom(e.to_string()))?,
+                .map_err(|e: CollectionError| E::custom(e.to_string()))?,
         ))
     }
 }

src/external/musicbrainz/api/search.rs

@@ -4,11 +4,8 @@ use serde::Deserialize;
 use url::form_urlencoded;
 
 use crate::{
-    collection::album::AlbumDate,
+    collection::{album::AlbumDate, musicbrainz::Mbid},
-    core::{
+    core::collection::album::{AlbumPrimaryType, AlbumSecondaryType},
-        collection::album::{AlbumPrimaryType, AlbumSecondaryType},
-        interface::musicbrainz::Mbid,
-    },
     external::musicbrainz::{
         api::{
             Error, MusicBrainzClient, SerdeAlbumDate, SerdeAlbumPrimaryType,

src/tui/app/machine/browse.rs

@@ -129,7 +129,7 @@ impl IAppInteractBrowse for AppMachine<AppBrowse> {
 #[cfg(test)]
 mod tests {
     use mockall::{predicate, Sequence};
-    use musichoard::{collection::album::Album, interface::musicbrainz::Mbid};
+    use musichoard::collection::{album::Album, musicbrainz::Mbid};
 
     use crate::tui::{
         app::{

src/tui/lib/external/musicbrainz/mod.rs

@@ -1,7 +1,10 @@
 //! Module for interacting with the [MusicBrainz API](https://musicbrainz.org/doc/MusicBrainz_API).
 
 use musichoard::{
-    collection::album::{Album, AlbumDate},
+    collection::{
+        album::{Album, AlbumDate},
+        musicbrainz::Mbid,
+    },
     external::musicbrainz::{
         api::{
             search::{SearchReleaseGroupRequest, SearchReleaseGroupResponseReleaseGroup},
@@ -9,7 +12,6 @@ use musichoard::{
         },
         IMusicBrainzHttp,
     },
-    interface::musicbrainz::Mbid,
 };
 
 use crate::tui::lib::interface::musicbrainz::{Error, IMusicBrainz, Match};

src/tui/lib/interface/musicbrainz/mod.rs

@@ -3,7 +3,7 @@
 #[cfg(test)]
 use mockall::automock;
 
-use musichoard::{collection::album::Album, interface::musicbrainz::Mbid};
+use musichoard::collection::{album::Album, musicbrainz::Mbid};
 
 /// Trait for interacting with the MusicBrainz API.
 #[cfg_attr(test, automock)]