Make MBID part of the artist identifier to disambiguate MB clashes (#241)
All checks were successful
Cargo CI / Build and Test (push) Successful in 2m0s
Cargo CI / Lint (push) Successful in 1m7s

And consistency with album identifier.

Part 3 of #231

Reviewed-on: #241
This commit is contained in:
Wojciech Kozlowski 2025-01-02 22:30:47 +01:00
parent dee8cbea4e
commit ccf139c2ca
23 changed files with 322 additions and 218 deletions

2
.gitattributes vendored
View File

@ -1 +1 @@
src/external/** linguist-vendored=false
**/external/** linguist-vendored=false

View File

@ -4,7 +4,7 @@ use std::{
};
use crate::core::collection::{
merge::{Merge, MergeSorted},
merge::{Merge, MergeName, MergeSorted},
musicbrainz::{MbAlbumRef, MbRefOption},
track::{Track, TrackFormat},
};
@ -57,6 +57,12 @@ impl AlbumLibId {
/// Unique database identifier. Use MBID for this purpose.
pub type AlbumDbId = MbRefOption<MbAlbumRef>;
impl MergeName for Album {
fn name(&self) -> &str {
&self.meta.id.title
}
}
// There are crates for handling dates, but we don't need much complexity beyond year-month-day.
/// The album's release date.
#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord)]
@ -305,7 +311,7 @@ impl AlbumId {
}
pub fn clear_db_id(&mut self) {
self.db_id = AlbumDbId::None;
self.db_id.take();
}
pub fn compatible(&self, other: &AlbumId) -> bool {

View File

@ -5,7 +5,7 @@ use std::{
use crate::core::collection::{
album::{Album, AlbumLibId},
merge::{Merge, MergeId},
merge::{Merge, MergeCollections, MergeName},
musicbrainz::{MbArtistRef, MbRefOption},
};
@ -25,17 +25,14 @@ pub struct ArtistMeta {
}
/// Artist non-identifier metadata.
#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, Default, PartialEq, Eq)]
pub struct ArtistInfo {
pub musicbrainz: MbRefOption<MbArtistRef>,
pub properties: HashMap<String, Vec<String>>,
}
impl MergeId for Artist {
type Id = ArtistId;
fn id(&self) -> &Self::Id {
&self.meta.id
impl MergeName for Artist {
fn name(&self) -> &str {
&self.meta.id.name
}
}
@ -43,8 +40,12 @@ impl MergeId for Artist {
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct ArtistId {
pub name: String,
pub db_id: ArtistDbId,
}
/// Unique database identifier. Use MBID for this purpose.
pub type ArtistDbId = MbRefOption<MbArtistRef>;
impl Artist {
/// Create new [`Artist`] with the given [`ArtistId`].
pub fn new<Id: Into<ArtistId>>(id: Id) -> Self {
@ -86,23 +87,6 @@ impl Artist {
}
secondary_without_id
}
fn merge_albums_by_title(
primary_albums: &mut Vec<Album>,
secondary_without_id: HashMap<String, Vec<Album>>,
) {
for (title, mut secondary_albums) in secondary_without_id.into_iter() {
let mut iter = primary_albums.iter_mut();
match iter.find(|album| album.meta.id.title == title) {
Some(ref mut primary_album) => {
// We do not support merging multiple DB albums with same title yet.
assert_eq!(secondary_albums.len(), 1);
primary_album.merge_in_place(secondary_albums.pop().unwrap())
}
None => primary_albums.append(&mut secondary_albums),
}
}
}
}
impl PartialOrd for Artist {
@ -122,7 +106,7 @@ impl Merge for Artist {
self.meta.merge_in_place(other.meta);
let other_without_id = Artist::merge_albums_by_lib_id(&mut self.albums, other.albums);
Artist::merge_albums_by_title(&mut self.albums, other_without_id);
MergeCollections::merge_by_name(&mut self.albums, other_without_id);
self.albums.sort_unstable();
}
}
@ -136,6 +120,14 @@ impl ArtistMeta {
}
}
pub fn set_db_id(&mut self, db_id: ArtistDbId) {
self.id.set_db_id(db_id);
}
pub fn clear_db_id(&mut self) {
self.id.clear_db_id();
}
pub fn get_sort_key(&self) -> (&str,) {
(self.sort.as_ref().unwrap_or(&self.id.name),)
}
@ -149,28 +141,7 @@ impl ArtistMeta {
}
}
impl Default for ArtistInfo {
fn default() -> Self {
Self::new(MbRefOption::None)
}
}
impl ArtistInfo {
pub fn new(musicbrainz: MbRefOption<MbArtistRef>) -> Self {
ArtistInfo {
musicbrainz,
properties: HashMap::new(),
}
}
pub fn set_musicbrainz_ref(&mut self, mbref: MbRefOption<MbArtistRef>) {
self.musicbrainz = mbref
}
pub fn clear_musicbrainz_ref(&mut self) {
self.musicbrainz.take();
}
// In the functions below, it would be better to use `contains` instead of `iter().any`, but for
// type reasons that does not work:
// https://stackoverflow.com/questions/48985924/why-does-a-str-not-coerce-to-a-string-when-using-veccontains
@ -230,8 +201,8 @@ impl Ord for ArtistMeta {
impl Merge for ArtistMeta {
fn merge_in_place(&mut self, other: Self) {
assert_eq!(self.id, other.id);
assert!(self.id.compatible(&other.id));
self.id.db_id = self.id.db_id.take().or(other.id.db_id);
self.sort = self.sort.take().or(other.sort);
self.info.merge_in_place(other.info);
}
@ -239,7 +210,6 @@ impl Merge for ArtistMeta {
impl Merge for ArtistInfo {
fn merge_in_place(&mut self, other: Self) {
self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz);
self.properties.merge_in_place(other.properties);
}
}
@ -258,7 +228,30 @@ impl AsRef<ArtistId> for ArtistId {
impl ArtistId {
pub fn new<S: Into<String>>(name: S) -> ArtistId {
ArtistId { name: name.into() }
ArtistId {
name: name.into(),
db_id: ArtistDbId::None,
}
}
pub fn with_db_id(mut self, db_id: ArtistDbId) -> Self {
self.db_id = db_id;
self
}
pub fn set_db_id(&mut self, db_id: ArtistDbId) {
self.db_id = db_id;
}
pub fn clear_db_id(&mut self) {
self.db_id.take();
}
pub fn compatible(&self, other: &ArtistId) -> bool {
let names_compatible = self.name == other.name;
let db_id_compatible =
self.db_id.is_none() || other.db_id.is_none() || (self.db_id == other.db_id);
names_compatible && db_id_compatible
}
}
@ -333,30 +326,30 @@ mod tests {
let mut artist = Artist::new(ArtistId::new("an artist"));
let mut expected: MbRefOption<MbArtistRef> = MbRefOption::None;
assert_eq!(artist.meta.info.musicbrainz, expected);
assert_eq!(artist.meta.id.db_id, expected);
// Setting a URL on an artist.
artist.meta.info.set_musicbrainz_ref(MbRefOption::Some(
artist.meta.id.set_db_id(MbRefOption::Some(
MbArtistRef::from_url_str(MUSICBRAINZ).unwrap(),
));
expected.replace(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap());
assert_eq!(artist.meta.info.musicbrainz, expected);
assert_eq!(artist.meta.id.db_id, expected);
artist.meta.info.set_musicbrainz_ref(MbRefOption::Some(
artist.meta.id.set_db_id(MbRefOption::Some(
MbArtistRef::from_url_str(MUSICBRAINZ).unwrap(),
));
assert_eq!(artist.meta.info.musicbrainz, expected);
assert_eq!(artist.meta.id.db_id, expected);
artist.meta.info.set_musicbrainz_ref(MbRefOption::Some(
artist.meta.id.set_db_id(MbRefOption::Some(
MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap(),
));
expected.replace(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap());
assert_eq!(artist.meta.info.musicbrainz, expected);
assert_eq!(artist.meta.id.db_id, expected);
// Clearing URLs.
artist.meta.info.clear_musicbrainz_ref();
artist.meta.id.clear_db_id();
expected.take();
assert_eq!(artist.meta.info.musicbrainz, expected);
assert_eq!(artist.meta.id.db_id, expected);
}
#[test]
@ -472,7 +465,7 @@ mod tests {
let left = FULL_COLLECTION[0].to_owned();
let mut right = FULL_COLLECTION[1].to_owned();
right.meta.id = left.meta.id.clone();
right.meta.info.musicbrainz = MbRefOption::None;
right.meta.id.db_id = MbRefOption::None;
right.meta.info.properties = HashMap::new();
let mut expected = left.clone();

View File

@ -80,36 +80,30 @@ where
}
}
pub trait MergeId {
type Id;
fn id(&self) -> &Self::Id;
pub trait MergeName {
fn name(&self) -> &str;
}
pub struct MergeCollections<ID, T, IT> {
_id: PhantomData<ID>,
pub struct MergeCollections<T, IT> {
_t: PhantomData<T>,
_it: PhantomData<IT>,
}
impl<ID, T, IT> MergeCollections<ID, T, IT>
impl<T, IT> MergeCollections<T, IT>
where
ID: Eq + Hash + Clone,
T: MergeId<Id = ID> + Merge + Ord,
IT: IntoIterator<Item = T>,
T: MergeName + Merge + Ord,
IT: IntoIterator<Item = (String, Vec<T>)>,
{
pub fn merge(mut primary: HashMap<ID, T>, secondary: IT) -> Vec<T> {
for secondary_item in secondary {
if let Some(ref mut primary_item) = primary.get_mut(secondary_item.id()) {
primary_item.merge_in_place(secondary_item);
} else {
primary.insert(secondary_item.id().clone(), secondary_item);
pub fn merge_by_name(primary_items: &mut Vec<T>, secondary: IT) {
for (name, mut secondary_items) in secondary.into_iter() {
match primary_items.iter_mut().find(|item| item.name() == name) {
Some(ref mut primary_item) => {
// We do not support merging multiple DB items with same name yet.
assert_eq!(secondary_items.len(), 1);
primary_item.merge_in_place(secondary_items.pop().unwrap());
}
None => primary_items.append(&mut secondary_items),
}
}
let mut collection: Vec<T> = primary.into_values().collect();
collection.sort_unstable();
collection
}
}

View File

@ -1,3 +1,5 @@
use std::collections::HashMap;
use crate::core::{
collection::{
album::{Album, AlbumId},
@ -57,7 +59,20 @@ impl<Database, Library> IMusicHoardBasePrivate for MusicHoard<Database, Library>
}
fn merge_collections(&self) -> Collection {
MergeCollections::merge(self.library_cache.clone(), self.database_cache.clone())
let mut primary = self.library_cache.clone();
let mut secondary = HashMap::<String, Vec<Artist>>::new();
for artist in self.database_cache.iter().cloned() {
secondary
.entry(artist.meta.id.name.clone())
.or_default()
.push(artist);
}
MergeCollections::merge_by_name(&mut primary, secondary);
primary.sort_unstable();
primary
}
fn get_artist<'a>(collection: &'a Collection, artist_id: &ArtistId) -> Option<&'a Artist> {
@ -118,11 +133,7 @@ mod tests {
expected.sort_unstable();
let mut mh = MusicHoard {
library_cache: left
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
library_cache: left.clone(),
database_cache: right.clone(),
..Default::default()
};
@ -132,11 +143,7 @@ mod tests {
// The merge is completely non-overlapping so it should be commutative.
let mut mh = MusicHoard {
library_cache: right
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
library_cache: right.clone(),
database_cache: left.clone(),
..Default::default()
};
@ -156,11 +163,7 @@ mod tests {
expected.sort_unstable();
let mut mh = MusicHoard {
library_cache: left
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
library_cache: left.clone(),
database_cache: right.clone(),
..Default::default()
};
@ -170,11 +173,7 @@ mod tests {
// The merge does not overwrite any data so it should be commutative.
let mut mh = MusicHoard {
library_cache: right
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
library_cache: right.clone(),
database_cache: left.clone(),
..Default::default()
};
@ -207,11 +206,7 @@ mod tests {
expected.rotate_right(1);
let mut mh = MusicHoard {
library_cache: left
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
library_cache: left.clone(),
database_cache: right.clone(),
..Default::default()
};
@ -221,11 +216,7 @@ mod tests {
// The merge overwrites the sort data, but no data is erased so it should be commutative.
let mut mh = MusicHoard {
library_cache: right
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
library_cache: right.clone(),
database_cache: left.clone(),
..Default::default()
};

View File

@ -1,5 +1,3 @@
use std::collections::HashMap;
use crate::core::{
interface::{database::IDatabase, library::ILibrary},
musichoard::{database::IMusicHoardDatabase, Error, MusicHoard, NoDatabase, NoLibrary},
@ -69,7 +67,7 @@ impl MusicHoard<NoDatabase, NoLibrary> {
database: NoDatabase,
database_cache: vec![],
library: NoLibrary,
library_cache: HashMap::new(),
library_cache: vec![],
}
}
}
@ -90,7 +88,7 @@ impl<Library: ILibrary> MusicHoard<NoDatabase, Library> {
database: NoDatabase,
database_cache: vec![],
library,
library_cache: HashMap::new(),
library_cache: vec![],
}
}
}
@ -111,7 +109,7 @@ impl<Database: IDatabase> MusicHoard<Database, NoLibrary> {
database,
database_cache: vec![],
library: NoLibrary,
library_cache: HashMap::new(),
library_cache: vec![],
};
mh.reload_database()?;
Ok(mh)
@ -134,7 +132,7 @@ impl<Database: IDatabase, Library: ILibrary> MusicHoard<Database, Library> {
database,
database_cache: vec![],
library,
library_cache: HashMap::new(),
library_cache: vec![],
};
mh.reload_database()?;
Ok(mh)

View File

@ -3,7 +3,7 @@ use std::mem;
use crate::{
collection::{
album::{AlbumDbId, AlbumInfo, AlbumMeta},
artist::ArtistInfo,
artist::{ArtistDbId, ArtistInfo},
merge::Merge,
},
core::{
@ -23,6 +23,13 @@ pub trait IMusicHoardDatabase {
fn add_artist<IntoId: Into<ArtistId>>(&mut self, artist_id: IntoId) -> Result<(), Error>;
fn remove_artist<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>;
fn set_artist_db_id<Id: AsRef<ArtistId>>(
&mut self,
artist_id: Id,
db_id: ArtistDbId,
) -> Result<(), Error>;
fn clear_artist_db_id<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>;
fn set_artist_sort<Id: AsRef<ArtistId>, S: Into<String>>(
&mut self,
artist_id: Id,
@ -140,6 +147,26 @@ impl<Database: IDatabase, Library> IMusicHoardDatabase for MusicHoard<Database,
})
}
fn set_artist_db_id<Id: AsRef<ArtistId>>(
&mut self,
artist_id: Id,
db_id: ArtistDbId,
) -> Result<(), Error> {
self.update_artist_and(
artist_id.as_ref(),
|artist| artist.meta.set_db_id(db_id),
|collection| Self::sort_artists(collection),
)
}
fn clear_artist_db_id<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error> {
self.update_artist_and(
artist_id.as_ref(),
|artist| artist.meta.clear_db_id(),
|collection| Self::sort_artists(collection),
)
}
fn set_artist_sort<Id: AsRef<ArtistId>, S: Into<String>>(
&mut self,
artist_id: Id,
@ -427,7 +454,7 @@ mod tests {
use crate::{
collection::{
album::{AlbumPrimaryType, AlbumSecondaryType},
musicbrainz::{MbArtistRef, MbRefOption},
musicbrainz::MbArtistRef,
},
core::{
collection::artist::ArtistId,
@ -557,6 +584,52 @@ mod tests {
assert_eq!(actual_err.to_string(), expected_err.to_string());
}
#[test]
fn set_clear_artist_db_id() {
let mut database = MockIDatabase::new();
database.expect_load().times(1).returning(|| Ok(vec![]));
database.expect_save().times(3).returning(|_| Ok(()));
let mut artist_id = ArtistId::new("an artist");
let artist_id_2 = ArtistId::new("another artist");
let mut music_hoard = MusicHoard::database(database).unwrap();
assert!(music_hoard.add_artist(artist_id.clone()).is_ok());
let mut expected = ArtistDbId::None;
assert_eq!(music_hoard.collection[0].meta.id.db_id, expected);
let db_id = ArtistDbId::Some(MbArtistRef::from_uuid_str(MBID).unwrap());
// Setting a db_id on an artist not in the collection is an error.
assert!(music_hoard
.set_artist_db_id(&artist_id_2, db_id.clone())
.is_err());
assert_eq!(music_hoard.collection[0].meta.id.db_id, expected);
// Setting a db_id on an artist.
assert!(music_hoard
.set_artist_db_id(&artist_id, db_id.clone())
.is_ok());
expected.replace(MbArtistRef::from_uuid_str(MBID).unwrap());
assert_eq!(music_hoard.collection[0].meta.id.db_id, expected);
// Clearing db_id on an artist that does not exist is an error.
assert!(music_hoard.clear_artist_db_id(&artist_id_2).is_err());
assert_eq!(music_hoard.collection[0].meta.id.db_id, expected);
// Clearing db_id from an artist without the db_id set is an error. Effectively the album
// does not exist.
assert!(music_hoard.clear_artist_db_id(&artist_id).is_err());
assert_eq!(music_hoard.collection[0].meta.id.db_id, expected);
// Clearing db_id.
artist_id.set_db_id(db_id);
assert!(music_hoard.clear_artist_db_id(&artist_id).is_ok());
expected.take();
assert_eq!(music_hoard.collection[0].meta.id.db_id, expected);
}
#[test]
fn set_clear_artist_info() {
let mut database = MockIDatabase::new();
@ -569,32 +642,36 @@ mod tests {
assert!(music_hoard.add_artist(artist_id.clone()).is_ok());
let mut expected: MbRefOption<MbArtistRef> = MbRefOption::None;
assert_eq!(music_hoard.collection[0].meta.info.musicbrainz, expected);
let mut expected = ArtistInfo::default();
assert_eq!(music_hoard.collection[0].meta.info, expected);
let info = ArtistInfo::new(MbRefOption::Some(MbArtistRef::from_uuid_str(MBID).unwrap()));
let mut info = ArtistInfo::default();
info.add_to_property("property", vec!["value-1", "value-2"]);
// Setting a URL on an artist not in the collection is an error.
// Setting info on an artist not in the collection is an error.
assert!(music_hoard
.merge_artist_info(&artist_id_2, info.clone())
.is_err());
assert_eq!(music_hoard.collection[0].meta.info.musicbrainz, expected);
assert_eq!(music_hoard.collection[0].meta.info, expected);
// Setting a URL on an artist.
// Setting info on an artist.
assert!(music_hoard
.merge_artist_info(&artist_id, info.clone())
.is_ok());
expected.replace(MbArtistRef::from_uuid_str(MBID).unwrap());
assert_eq!(music_hoard.collection[0].meta.info.musicbrainz, expected);
expected.properties.insert(
String::from("property"),
vec![String::from("value-1"), String::from("value-2")],
);
assert_eq!(music_hoard.collection[0].meta.info, expected);
// Clearing URLs on an artist that does not exist is an error.
// Clearing info on an artist that does not exist is an error.
assert!(music_hoard.clear_artist_info(&artist_id_2).is_err());
assert_eq!(music_hoard.collection[0].meta.info.musicbrainz, expected);
assert_eq!(music_hoard.collection[0].meta.info, expected);
// Clearing URLs.
// Clearing info.
assert!(music_hoard.clear_artist_info(&artist_id).is_ok());
expected.take();
assert_eq!(music_hoard.collection[0].meta.info.musicbrainz, expected);
expected.properties.clear();
assert_eq!(music_hoard.collection[0].meta.info, expected);
}
#[test]
@ -786,8 +863,8 @@ mod tests {
.clear_album_db_id(&artist_id, &album_id_2)
.is_err());
// Clearing db_id from album without the db_id set is an error. Effectively the album does
// not exist.
// Clearing db_id from an album without the db_id set is an error. Effectively the album
// does not exist.
assert!(music_hoard
.clear_album_db_id(&artist_id, &album_id)
.is_err());

View File

@ -1,7 +1,7 @@
use std::collections::HashMap;
use crate::{
collection::album::AlbumDbId,
collection::{album::AlbumDbId, artist::ArtistDbId},
core::{
collection::{
album::{Album, AlbumDate, AlbumId},
@ -42,17 +42,18 @@ impl<Database, Library: ILibrary> MusicHoard<Database, Library> {
fn rescan_library_inner(&mut self) -> Result<Collection, Error> {
let items = self.library.list(&Query::new())?;
self.library_cache = Self::items_to_artists(items)?;
Self::sort_albums_and_tracks(self.library_cache.values_mut());
Self::sort_albums_and_tracks(self.library_cache.iter_mut());
Ok(self.merge_collections())
}
fn items_to_artists(items: Vec<Item>) -> Result<HashMap<ArtistId, Artist>, Error> {
fn items_to_artists(items: Vec<Item>) -> Result<Collection, Error> {
let mut collection = HashMap::<ArtistId, Artist>::new();
for item in items.into_iter() {
let artist_id = ArtistId {
name: item.album_artist,
db_id: ArtistDbId::None,
};
let artist_sort = item.album_artist_sort;
@ -121,7 +122,7 @@ impl<Database, Library: ILibrary> MusicHoard<Database, Library> {
}
}
Ok(collection)
Ok(collection.into_values().collect())
}
}

View File

@ -10,15 +10,9 @@ pub use base::IMusicHoardBase;
pub use database::IMusicHoardDatabase;
pub use library::IMusicHoardLibrary;
use std::{
collections::HashMap,
fmt::{self, Display},
};
use std::fmt::{self, Display};
use crate::core::collection::{
artist::{Artist, ArtistId},
Collection,
};
use crate::core::collection::Collection;
use crate::core::interface::{
database::{LoadError as DatabaseLoadError, SaveError as DatabaseSaveError},
@ -35,7 +29,7 @@ pub struct MusicHoard<Database, Library> {
database: Database,
database_cache: Collection,
library: Library,
library_cache: HashMap<ArtistId, Artist>,
library_cache: Collection,
}
/// Phantom type for when a library implementation is not needed.

View File

@ -5,8 +5,8 @@ use crate::core::collection::{
album::{
Album, AlbumDbId, AlbumId, AlbumInfo, AlbumLibId, AlbumMeta, AlbumPrimaryType, AlbumSeq,
},
artist::{Artist, ArtistId, ArtistInfo, ArtistMeta},
musicbrainz::{MbAlbumRef, MbArtistRef, MbRefOption},
artist::{Artist, ArtistDbId, ArtistId, ArtistInfo, ArtistMeta},
musicbrainz::{MbAlbumRef, MbArtistRef},
track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality},
};
use crate::testmod::*;

View File

@ -123,10 +123,12 @@ impl From<DeserializeArtist> for Artist {
fn from(artist: DeserializeArtist) -> Self {
Artist {
meta: ArtistMeta {
id: ArtistId::new(artist.name),
id: ArtistId {
name: artist.name,
db_id: artist.musicbrainz.into(),
},
sort: artist.sort,
info: ArtistInfo {
musicbrainz: artist.musicbrainz.into(),
properties: artist.properties,
},
},

View File

@ -86,7 +86,7 @@ impl<'a> From<&'a Artist> for SerializeArtist<'a> {
SerializeArtist {
name: &artist.meta.id.name,
sort: artist.meta.sort.as_deref(),
musicbrainz: (&artist.meta.info.musicbrainz).into(),
musicbrainz: (&artist.meta.id.db_id).into(),
properties: artist
.meta
.info

View File

@ -5,12 +5,12 @@ macro_rules! full_collection {
meta: ArtistMeta {
id: ArtistId {
name: "Album_Artist A".to_string(),
db_id: ArtistDbId::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000"
).unwrap()),
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/000000000"),
@ -135,12 +135,12 @@ macro_rules! full_collection {
meta: ArtistMeta {
id: ArtistId {
name: "Album_Artist B".to_string(),
db_id: ArtistDbId::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111"
).unwrap()),
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/111111111"),
@ -336,10 +336,10 @@ macro_rules! full_collection {
meta: ArtistMeta {
id: ArtistId {
name: "The Album_Artist C".to_string(),
db_id: ArtistDbId::CannotHaveMbid,
},
sort: Some("Album_Artist C, The".to_string()),
info: ArtistInfo {
musicbrainz: MbRefOption::CannotHaveMbid,
properties: HashMap::new(),
},
},
@ -434,10 +434,10 @@ macro_rules! full_collection {
meta: ArtistMeta {
id: ArtistId {
name: "Album_Artist D".to_string(),
db_id: ArtistDbId::None,
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::None,
properties: HashMap::new(),
},
},

View File

@ -6,10 +6,10 @@ macro_rules! library_collection {
meta: ArtistMeta {
id: ArtistId {
name: "Album_Artist A".to_string(),
db_id: ArtistDbId::None,
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::None,
properties: HashMap::new(),
},
},
@ -117,10 +117,10 @@ macro_rules! library_collection {
meta: ArtistMeta {
id: ArtistId {
name: "Album_Artist B".to_string(),
db_id: ArtistDbId::None,
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::None,
properties: HashMap::new(),
},
},
@ -287,10 +287,10 @@ macro_rules! library_collection {
meta: ArtistMeta {
id: ArtistId {
name: "The Album_Artist C".to_string(),
db_id: ArtistDbId::None,
},
sort: Some("Album_Artist C, The".to_string()),
info: ArtistInfo {
musicbrainz: MbRefOption::None,
properties: HashMap::new(),
},
},
@ -379,10 +379,10 @@ macro_rules! library_collection {
meta: ArtistMeta {
id: ArtistId {
name: "Album_Artist D".to_string(),
db_id: ArtistDbId::None,
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::None,
properties: HashMap::new(),
},
},

View File

@ -115,14 +115,14 @@ impl AppMachine<FetchState> {
let mut requests = Self::search_artist_job(artist);
if requests.is_empty() {
fetch = FetchState::fetch(rx);
requests = Self::browse_release_group_job(&artist.meta.info.musicbrainz);
requests = Self::browse_release_group_job(&artist.meta.id.db_id);
} else {
fetch = FetchState::search(rx);
}
SubmitJob { fetch, requests }
}
_ => {
let arid = match artist.meta.info.musicbrainz {
let arid = match artist.meta.id.db_id {
MbRefOption::Some(ref mbref) => mbref,
_ => return Err("cannot fetch album: artist has no MBID"),
};
@ -258,7 +258,7 @@ impl AppMachine<FetchState> {
}
fn search_artist_job(artist: &Artist) -> VecDeque<MbParams> {
match artist.meta.info.musicbrainz {
match artist.meta.id.db_id {
MbRefOption::Some(ref arid) => {
Self::search_albums_requests(&artist.meta.id, arid, &artist.albums)
}
@ -784,7 +784,7 @@ mod tests {
}
fn browse_release_group_expectation(artist: &Artist) -> MockIMbJobSender {
let requests = AppMachine::browse_release_group_job(&artist.meta.info.musicbrainz);
let requests = AppMachine::browse_release_group_job(&artist.meta.id.db_id);
let mut mb_job_sender = MockIMbJobSender::new();
mb_job_sender
.expect_submit_background_job()

View File

@ -2,7 +2,7 @@ use std::cmp;
use musichoard::collection::{
album::{AlbumDbId, AlbumInfo, AlbumMeta},
artist::{ArtistInfo, ArtistMeta},
artist::{ArtistDbId, ArtistInfo, ArtistMeta},
musicbrainz::{MbRefOption, Mbid},
};
@ -12,6 +12,11 @@ use crate::tui::app::{
MatchOption, MatchStatePublic, WidgetState,
};
struct ArtistInfoTuple {
db_id: ArtistDbId,
info: ArtistInfo,
}
struct AlbumInfoTuple {
db_id: AlbumDbId,
info: AlbumInfo,
@ -21,7 +26,7 @@ trait GetInfoMeta {
type InfoType;
}
impl GetInfoMeta for ArtistMeta {
type InfoType = ArtistInfo;
type InfoType = ArtistInfoTuple;
}
impl GetInfoMeta for AlbumMeta {
type InfoType = AlbumInfoTuple;
@ -38,16 +43,20 @@ enum InfoOption<T> {
}
impl GetInfo for MatchOption<ArtistMeta> {
type InfoType = ArtistInfo;
type InfoType = ArtistInfoTuple;
fn get_info(&self) -> InfoOption<Self::InfoType> {
let db_id;
let mut info = ArtistInfo::default();
match self {
MatchOption::Some(option) => info.musicbrainz = option.entity.info.musicbrainz.clone(),
MatchOption::CannotHaveMbid => info.musicbrainz = MbRefOption::CannotHaveMbid,
MatchOption::Some(option) => {
db_id = option.entity.id.db_id.clone();
info = option.entity.info.clone();
}
MatchOption::CannotHaveMbid => db_id = MbRefOption::CannotHaveMbid,
MatchOption::ManualInputMbid => return InfoOption::NeedInput,
}
InfoOption::Info(info)
InfoOption::Info(ArtistInfoTuple { db_id, info })
}
}
@ -237,7 +246,9 @@ impl IAppInteractMatch for AppMachine<MatchState> {
let mh = &mut self.inner.music_hoard;
let result = match self.state.current {
EntityMatches::Artist(ref mut matches) => match matches.list.extract_info(index) {
InfoOption::Info(info) => mh.merge_artist_info(&matches.matching.id, info),
InfoOption::Info(tuple) => mh
.merge_artist_info(&matches.matching.id, tuple.info)
.and_then(|()| mh.set_artist_db_id(&matches.matching.id, tuple.db_id)),
InfoOption::NeedInput => return self.get_input(),
},
EntityMatches::Album(ref mut matches) => match matches.list.extract_info(index) {
@ -303,13 +314,11 @@ mod tests {
}
fn artist_meta() -> ArtistMeta {
let mut meta = ArtistMeta::new(ArtistId::new("Artist"));
meta.info.musicbrainz = MbRefOption::Some(mbid().into());
meta
ArtistMeta::new(ArtistId::new("Artist").with_db_id(ArtistDbId::Some(mbid().into())))
}
fn artist_match() -> EntityMatches {
let artist = artist_meta();
let mut artist = artist_meta();
let artist_1 = artist.clone();
let artist_match_1 = Entity::with_score(artist_1, 100);
@ -318,12 +327,14 @@ mod tests {
let mut artist_match_2 = Entity::with_score(artist_2, 100);
artist_match_2.disambiguation = Some(String::from("some disambiguation"));
artist.clear_db_id();
let list = vec![artist_match_1.clone(), artist_match_2.clone()];
EntityMatches::artist_search(artist, list)
}
fn artist_lookup() -> EntityMatches {
let artist = artist_meta();
let mut artist = artist_meta();
artist.clear_db_id();
let lookup = Entity::new(artist.clone());
EntityMatches::artist_lookup(artist, lookup)
}
@ -427,14 +438,21 @@ mod tests {
.return_once(|_, _, _| Ok(()));
}
EntityMatches::Artist(_) => {
let info = ArtistInfo {
musicbrainz: MbRefOption::CannotHaveMbid,
..Default::default()
};
let db_id = MbRefOption::CannotHaveMbid;
let info = ArtistInfo::default();
let mut seq = Sequence::new();
music_hoard
.expect_merge_artist_info()
.with(eq(artist_id.clone()), eq(info))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _| Ok(()));
music_hoard
.expect_set_artist_db_id()
.with(eq(artist_id.clone()), eq(db_id))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _| Ok(()));
}
}
@ -514,11 +532,22 @@ mod tests {
match matches_info {
EntityMatches::Album(_) => panic!(),
EntityMatches::Artist(_) => {
let meta = artist_meta();
let mut meta = artist_meta();
let db_id = meta.id.db_id.clone();
meta.clear_db_id();
let mut seq = Sequence::new();
music_hoard
.expect_merge_artist_info()
.with(eq(meta.id), eq(meta.info))
.with(eq(meta.id.clone()), eq(meta.info))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _| Ok(()));
music_hoard
.expect_set_artist_db_id()
.with(eq(meta.id.clone()), eq(db_id))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _| Ok(()));
}
}

View File

@ -5,7 +5,7 @@ use std::collections::HashMap;
use musichoard::{
collection::{
album::{AlbumDate, AlbumDbId, AlbumId, AlbumInfo, AlbumLibId, AlbumMeta, AlbumSeq},
artist::{ArtistInfo, ArtistMeta},
artist::{ArtistId, ArtistInfo, ArtistMeta},
musicbrainz::{MbRefOption, Mbid},
},
external::musicbrainz::{
@ -115,10 +115,12 @@ fn from_mb_artist_meta(meta: MbArtistMeta) -> (ArtistMeta, Option<String>) {
let sort = Some(meta.sort_name).filter(|s| s != &meta.name);
(
ArtistMeta {
id: meta.name.into(),
id: ArtistId {
name: meta.name,
db_id: MbRefOption::Some(meta.id.into()),
},
sort,
info: ArtistInfo {
musicbrainz: MbRefOption::Some(meta.id.into()),
properties: HashMap::new(),
},
},

View File

@ -454,7 +454,7 @@ mod tests {
}
fn search_albums_requests() -> VecDeque<MbParams> {
let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.info.musicbrainz);
let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.id.db_id);
let arid = mb_ref_opt_unwrap(mbref).mbid().clone();
let artist_id = COLLECTION[1].meta.id.clone();
@ -468,7 +468,7 @@ mod tests {
}
fn browse_albums_requests() -> VecDeque<MbParams> {
let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.info.musicbrainz);
let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.id.db_id);
let arid = mb_ref_opt_unwrap(mbref).mbid().clone();
VecDeque::from([MbParams::browse_release_group(arid)])
}
@ -478,7 +478,7 @@ mod tests {
}
fn album_arid_expectation() -> Mbid {
let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.info.musicbrainz);
let mbref = mb_ref_opt_as_ref(&COLLECTION[1].meta.id.db_id);
mb_ref_opt_unwrap(mbref).mbid().clone()
}

View File

@ -29,6 +29,10 @@ impl fmt::Display for Error {
pub type MbApiResult = Result<MbReturn, MbApiError>;
pub type ResultSender = mpsc::Sender<MbApiResult>;
// The largest variant is Match(EntityMatches::Album(AlbumMatches)) since that contains both the
// ArtistId and Albumid. However, that is also the most common MbReturn expected. Therefore, we
// allow this large enum variant.
#[allow(clippy::large_enum_variant)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub enum MbReturn {
Match(EntityMatches),

View File

@ -4,7 +4,7 @@ pub mod interface;
use musichoard::{
collection::{
album::{AlbumDbId, AlbumId, AlbumInfo, AlbumMeta},
artist::{ArtistId, ArtistInfo},
artist::{ArtistDbId, ArtistId, ArtistInfo},
Collection,
},
interface::{database::IDatabase, library::ILibrary},
@ -26,6 +26,11 @@ pub trait IMusicHoard {
album_meta: AlbumMeta,
) -> Result<(), musichoard::Error>;
fn set_artist_db_id(
&mut self,
artist_id: &ArtistId,
db_id: ArtistDbId,
) -> Result<(), musichoard::Error>;
fn merge_artist_info(
&mut self,
id: &ArtistId,
@ -67,6 +72,14 @@ impl<Database: IDatabase, Library: ILibrary> IMusicHoard for MusicHoard<Database
<Self as IMusicHoardDatabase>::add_album(self, artist_id, album_meta)
}
fn set_artist_db_id(
&mut self,
artist_id: &ArtistId,
db_id: ArtistDbId,
) -> Result<(), musichoard::Error> {
<Self as IMusicHoardDatabase>::set_artist_db_id(self, artist_id, db_id)
}
fn merge_artist_info(
&mut self,
id: &ArtistId,

View File

@ -4,8 +4,8 @@ use musichoard::collection::{
album::{
Album, AlbumDbId, AlbumId, AlbumInfo, AlbumLibId, AlbumMeta, AlbumPrimaryType, AlbumSeq,
},
artist::{Artist, ArtistId, ArtistInfo, ArtistMeta},
musicbrainz::{MbAlbumRef, MbArtistRef, MbRefOption},
artist::{Artist, ArtistDbId, ArtistId, ArtistInfo, ArtistMeta},
musicbrainz::{MbAlbumRef, MbArtistRef},
track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality},
};
use once_cell::sync::Lazy;

View File

@ -76,7 +76,7 @@ impl<'a> ArtistOverlay<'a> {
Properties: {}",
artist.map(|a| a.meta.id.name.as_str()).unwrap_or(""),
artist
.map(|a| UiDisplay::display_mb_ref_option_as_url(&a.meta.info.musicbrainz))
.map(|a| UiDisplay::display_mb_ref_option_as_url(&a.meta.id.db_id))
.unwrap_or_default(),
Self::opt_hashmap_to_string(
artist.map(|a| &a.meta.info.properties),

View File

@ -6,8 +6,8 @@ use musichoard::collection::{
Album, AlbumDbId, AlbumId, AlbumInfo, AlbumLibId, AlbumMeta, AlbumPrimaryType,
AlbumSecondaryType, AlbumSeq,
},
artist::{Artist, ArtistId, ArtistInfo, ArtistMeta},
musicbrainz::{MbArtistRef, MbRefOption},
artist::{Artist, ArtistDbId, ArtistId, ArtistInfo, ArtistMeta},
musicbrainz::MbArtistRef,
track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality},
Collection,
};
@ -18,12 +18,12 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
meta: ArtistMeta {
id: ArtistId {
name: String::from("Аркона"),
db_id: ArtistDbId::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212"
).unwrap()),
},
sort: Some(String::from("Arkona")),
info: ArtistInfo {
musicbrainz: MbRefOption::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/283448581"),
@ -213,12 +213,12 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
meta: ArtistMeta {
id: ArtistId {
name: String::from("Eluveitie"),
db_id: ArtistDbId::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38"
).unwrap()),
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/269358403"),
@ -468,12 +468,12 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
meta: ArtistMeta {
id: ArtistId {
name: String::from("Frontside"),
db_id: ArtistDbId::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490"
).unwrap()),
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/826588800"),
@ -627,12 +627,12 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
meta: ArtistMeta {
id: ArtistId {
name: String::from("Heavens Basement"),
db_id: ArtistDbId::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc"
).unwrap()),
},
sort: Some(String::from("Heavens Basement")),
info: ArtistInfo {
musicbrainz: MbRefOption::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/291158685"),
@ -766,12 +766,12 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
meta: ArtistMeta {
id: ArtistId {
name: String::from("Metallica"),
db_id: ArtistDbId::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab"
).unwrap()),
},
sort: None,
info: ArtistInfo {
musicbrainz: MbRefOption::Some(MbArtistRef::from_url_str(
"https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
String::from("https://www.musicbutler.io/artist-page/3996865"),