The MusicBrainz API search call should use the MBID if available (#171)

Closes #169

Reviewed-on: #171

examples/musicbrainz_api/search_release_group.rs

@@ -21,6 +21,20 @@ struct Opt {
     #[structopt(help = "Release group's artist MBID")]
     arid: Uuid,
 
+    #[structopt(subcommand)]
+    command: OptCommand,
+}
+
+#[derive(StructOpt)]
+enum OptCommand {
+    #[structopt(about = "Search by title (and date)")]
+    Title(OptTitle),
+    #[structopt(about = "Search by release group MBID")]
+    Rgid(OptRgid),
+}
+
+#[derive(StructOpt)]
+struct OptTitle {
     #[structopt(help = "Release group title")]
     title: String,
 
@@ -28,6 +42,12 @@ struct Opt {
     date: Option<Date>,
 }
 
+#[derive(StructOpt)]
+struct OptRgid {
+    #[structopt(help = "Release group MBID")]
+    rgid: Uuid,
+}
+
 struct Date(AlbumDate);
 
 impl FromStr for Date {
@@ -64,10 +84,26 @@ fn main() {
     let mut api = MusicBrainzApi::new(client);
 
     let arid: Mbid = opt.arid.into();
-    let date: AlbumDate = opt.date.map(Into::into).unwrap_or_default();
+
-    let album = Album::new(AlbumId::new(opt.title), date, None, vec![]);
+    let album = match opt.command {
+        OptCommand::Title(opt_title) => {
+            let date: AlbumDate = opt_title.date.map(Into::into).unwrap_or_default();
+            Album::new(AlbumId::new(opt_title.title), date, None, vec![])
+        }
+        OptCommand::Rgid(opt_rgid) => {
+            let mut album = Album::new(
+                AlbumId::new(String::default()),
+                AlbumDate::default(),
+                None,
+                vec![],
+            );
+            album.set_musicbrainz_ref(opt_rgid.rgid.into());
+            album
+        }
+    };
+
     let matches = api
-        .search_release_group(&arid, album)
+        .search_release_group(&arid, &album)
         .expect("failed to make API call");
 
     println!("{matches:#?}");

src/core/interface/musicbrainz/mod.rs

@@ -16,7 +16,7 @@ pub trait IMusicBrainz {
     fn search_release_group(
         &mut self,
         arid: &Mbid,
-        album: Album,
+        album: &Album,
     ) -> Result<Vec<Match<Album>>, Error>;
 }
 
@@ -44,7 +44,7 @@ impl IMusicBrainz for NullMusicBrainz {
     fn search_release_group(
         &mut self,
         _arid: &Mbid,
-        _album: Album,
+        _album: &Album,
     ) -> Result<Vec<Match<Album>>, Error> {
         Ok(vec![])
     }
@@ -143,7 +143,7 @@ mod tests {
         let mbid: Mbid = "d368baa8-21ca-4759-9731-0b2753071ad8".try_into().unwrap();
         let album = Album::new(AlbumId::new("an album"), AlbumDate::default(), None, vec![]);
         assert!(musicbrainz
-            .search_release_group(&mbid, album)
+            .search_release_group(&mbid, &album)
             .unwrap()
             .is_empty());
     }

src/external/musicbrainz/api/mod.rs

@@ -11,7 +11,7 @@ use mockall::automock;
 use crate::core::{
     collection::{
         album::{Album, AlbumDate, AlbumPrimaryType, AlbumSecondaryType},
-        musicbrainz::MbAlbumRef,
+        musicbrainz::{IMusicBrainzRef, MbAlbumRef},
     },
     interface::musicbrainz::{Error, IMusicBrainz, Match, Mbid},
 };
@@ -70,14 +70,23 @@ impl<Mbc: IMusicBrainzApiClient> IMusicBrainz for MusicBrainzApi<Mbc> {
     fn search_release_group(
         &mut self,
         arid: &Mbid,
-        album: Album,
+        album: &Album,
     ) -> Result<Vec<Match<Album>>, Error> {
         let title = &album.id.title;
         let arid = arid.uuid().as_hyphenated().to_string();
-        let mut query = format!("releasegroup:\"{title}\" AND arid:{arid}");
+        let mut query = format!("arid:{arid}");
 
-        if let Some(year) = album.date.year {
+        match album.musicbrainz {
-            query.push_str(&format!(" AND firstreleasedate:{year}"));
+            Some(ref mbref) => {
+                let rgid = mbref.mbid().uuid().as_hyphenated().to_string();
+                query.push_str(&format!(" AND rgid:{rgid}"));
+            }
+            None => {
+                query.push_str(&format!(" AND releasegroup:\"{title}\""));
+                if let Some(year) = album.date.year {
+                    query.push_str(&format!(" AND firstreleasedate:{year}"));
+                }
+            }
         }
 
         let query: String = form_urlencoded::byte_serialize(query.as_bytes()).collect();
@@ -127,13 +136,13 @@ impl TryFrom<LookupReleaseGroup> for Album {
     }
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 #[serde(rename_all(deserialize = "kebab-case"))]
 struct ResponseSearchReleaseGroup {
     release_groups: Vec<SearchReleaseGroup>,
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 #[serde(rename_all(deserialize = "kebab-case"))]
 struct SearchReleaseGroup {
     score: u8,
@@ -190,7 +199,7 @@ pub enum SerdeAlbumPrimaryTypeDef {
     Other,
 }
 
-#[derive(Debug, Deserialize)]
+#[derive(Clone, Debug, Deserialize)]
 pub struct SerdeAlbumPrimaryType(#[serde(with = "SerdeAlbumPrimaryTypeDef")] AlbumPrimaryType);
 
 impl From<SerdeAlbumPrimaryType> for AlbumPrimaryType {
@@ -233,7 +242,7 @@ impl From<SerdeAlbumSecondaryType> for AlbumSecondaryType {
 
 #[cfg(test)]
 mod tests {
-    use mockall::predicate;
+    use mockall::{predicate, Sequence};
 
     use crate::collection::album::AlbumId;
 
@@ -289,14 +298,21 @@ mod tests {
     #[test]
     fn search_release_group() {
         let mut client = MockIMusicBrainzApiClient::new();
-        let url = format!(
+        let url_title = format!(
             "https://musicbrainz.org/ws/2\
              /release-group\
-             ?query=releasegroup%3A%22{title}%22+AND+arid%3A{arid}+AND+firstreleasedate%3A{year}",
+             ?query=arid%3A{arid}+AND+releasegroup%3A%22{title}%22+AND+firstreleasedate%3A{year}",
-            title = "an+album",
             arid = "00000000-0000-0000-0000-000000000000",
+            title = "an+album",
             year = "1986"
         );
+        let url_rgid = format!(
+            "https://musicbrainz.org/ws/2\
+             /release-group\
+             ?query=arid%3A{arid}+AND+rgid%3A{rgid}",
+            arid = "00000000-0000-0000-0000-000000000000",
+            rgid = "11111111-1111-1111-1111-111111111111",
+        );
 
         let release_group = SearchReleaseGroup {
             score: 67,
@@ -312,17 +328,23 @@ mod tests {
         // For code coverage of derive(Debug).
         assert!(!format!("{response:?}").is_empty());
 
+        let mut seq = Sequence::new();
+
+        let title_response = response.clone();
         client
             .expect_get()
             .times(1)
-            .with(predicate::eq(url))
+            .with(predicate::eq(url_title))
-            .return_once(|_| Ok(response));
+            .return_once(|_| Ok(title_response))
+            .in_sequence(&mut seq);
 
-        let mut api = MusicBrainzApi::new(client);
+        let rgid_response = response;
-
+        client
-        let arid: Mbid = "00000000-0000-0000-0000-000000000000".try_into().unwrap();
+            .expect_get()
-        let album = Album::new(AlbumId::new("an album"), (1986, 4), None, vec![]);
+            .times(1)
-        let matches = api.search_release_group(&arid, album).unwrap();
+            .with(predicate::eq(url_rgid))
+            .return_once(|_| Ok(rgid_response))
+            .in_sequence(&mut seq);
 
         let mut album = Album::new(
             AlbumId::new("an album"),
@@ -335,6 +357,16 @@ mod tests {
         );
         let expected = vec![Match::new(67, album)];
 
+        let mut api = MusicBrainzApi::new(client);
+
+        let arid: Mbid = "00000000-0000-0000-0000-000000000000".try_into().unwrap();
+        let mut album = Album::new(AlbumId::new("an album"), (1986, 4), None, vec![]);
+        let matches = api.search_release_group(&arid, &album).unwrap();
+        assert_eq!(matches, expected);
+
+        let rgid = MbAlbumRef::from_uuid_str("11111111-1111-1111-1111-111111111111").unwrap();
+        album.set_musicbrainz_ref(rgid);
+        let matches = api.search_release_group(&arid, &album).unwrap();
         assert_eq!(matches, expected);
     }