Make MBID part of the artist identifier to disambiguate MB clashes #241

Merged
wojtek merged 6 commits from 231---differentiate-release-groups-with-same-title-but-different-type-clash into main 2025-01-02 22:30:48 +01:00
6 changed files with 51 additions and 76 deletions
Showing only changes of commit eccec67592 - Show all commits

View File

@ -5,7 +5,7 @@ use std::{
use crate::core::collection::{ use crate::core::collection::{
album::{Album, AlbumLibId}, album::{Album, AlbumLibId},
merge::{Merge, MergeId}, merge::{Merge, MergeName},
musicbrainz::{MbArtistRef, MbRefOption}, musicbrainz::{MbArtistRef, MbRefOption},
}; };
@ -30,11 +30,9 @@ pub struct ArtistInfo {
pub properties: HashMap<String, Vec<String>>, pub properties: HashMap<String, Vec<String>>,
} }
impl MergeId for Artist { impl MergeName for Artist {
type Id = ArtistId; fn name(&self) -> &str {
&self.meta.id.name
fn id(&self) -> &Self::Id {
&self.meta.id
} }
} }

View File

@ -80,36 +80,30 @@ where
} }
} }
pub trait MergeId { pub trait MergeName {
type Id; fn name(&self) -> &str;
fn id(&self) -> &Self::Id;
} }
pub struct MergeCollections<ID, T, IT> { pub struct MergeCollections<T, IT> {
_id: PhantomData<ID>,
_t: PhantomData<T>, _t: PhantomData<T>,
_it: PhantomData<IT>, _it: PhantomData<IT>,
} }
impl<ID, T, IT> MergeCollections<ID, T, IT> impl<T, IT> MergeCollections<T, IT>
where where
ID: Eq + Hash + Clone, T: MergeName + Merge + Ord,
T: MergeId<Id = ID> + Merge + Ord, IT: IntoIterator<Item = (String, Vec<T>)>,
IT: IntoIterator<Item = T>,
{ {
pub fn merge(mut primary: HashMap<ID, T>, secondary: IT) -> Vec<T> { pub fn merge_by_name(primary_items: &mut Vec<T>, secondary: IT) {
for secondary_item in secondary { for (name, mut secondary_items) in secondary.into_iter() {
if let Some(ref mut primary_item) = primary.get_mut(secondary_item.id()) { match primary_items.iter_mut().find(|item| item.name() == &name) {
primary_item.merge_in_place(secondary_item); Some(ref mut primary_item) => {
} else { // We do not support merging multiple DB items with same name yet.
primary.insert(secondary_item.id().clone(), secondary_item); 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::{ use crate::core::{
collection::{ collection::{
album::{Album, AlbumId}, album::{Album, AlbumId},
@ -57,7 +59,20 @@ impl<Database, Library> IMusicHoardBasePrivate for MusicHoard<Database, Library>
} }
fn merge_collections(&self) -> Collection { 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> { fn get_artist<'a>(collection: &'a Collection, artist_id: &ArtistId) -> Option<&'a Artist> {
@ -118,11 +133,7 @@ mod tests {
expected.sort_unstable(); expected.sort_unstable();
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left library_cache: left.clone(),
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
database_cache: right.clone(), database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
@ -132,11 +143,7 @@ mod tests {
// The merge is completely non-overlapping so it should be commutative. // The merge is completely non-overlapping so it should be commutative.
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: right library_cache: right.clone(),
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
database_cache: left.clone(), database_cache: left.clone(),
..Default::default() ..Default::default()
}; };
@ -156,11 +163,7 @@ mod tests {
expected.sort_unstable(); expected.sort_unstable();
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left library_cache: left.clone(),
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
database_cache: right.clone(), database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
@ -170,11 +173,7 @@ mod tests {
// The merge does not overwrite any data so it should be commutative. // The merge does not overwrite any data so it should be commutative.
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: right library_cache: right.clone(),
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
database_cache: left.clone(), database_cache: left.clone(),
..Default::default() ..Default::default()
}; };
@ -207,11 +206,7 @@ mod tests {
expected.rotate_right(1); expected.rotate_right(1);
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: left library_cache: left.clone(),
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
database_cache: right.clone(), database_cache: right.clone(),
..Default::default() ..Default::default()
}; };
@ -221,11 +216,7 @@ mod tests {
// The merge overwrites the sort data, but no data is erased so it should be commutative. // The merge overwrites the sort data, but no data is erased so it should be commutative.
let mut mh = MusicHoard { let mut mh = MusicHoard {
library_cache: right library_cache: right.clone(),
.clone()
.into_iter()
.map(|a| (a.meta.id.clone(), a))
.collect(),
database_cache: left.clone(), database_cache: left.clone(),
..Default::default() ..Default::default()
}; };

View File

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

View File

@ -42,12 +42,12 @@ impl<Database, Library: ILibrary> MusicHoard<Database, Library> {
fn rescan_library_inner(&mut self) -> Result<Collection, Error> { fn rescan_library_inner(&mut self) -> Result<Collection, Error> {
let items = self.library.list(&Query::new())?; let items = self.library.list(&Query::new())?;
self.library_cache = Self::items_to_artists(items)?; 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()) 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(); let mut collection = HashMap::<ArtistId, Artist>::new();
for item in items.into_iter() { for item in items.into_iter() {
@ -122,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 database::IMusicHoardDatabase;
pub use library::IMusicHoardLibrary; pub use library::IMusicHoardLibrary;
use std::{ use std::fmt::{self, Display};
collections::HashMap,
fmt::{self, Display},
};
use crate::core::collection::{ use crate::core::collection::Collection;
artist::{Artist, ArtistId},
Collection,
};
use crate::core::interface::{ use crate::core::interface::{
database::{LoadError as DatabaseLoadError, SaveError as DatabaseSaveError}, database::{LoadError as DatabaseLoadError, SaveError as DatabaseSaveError},
@ -35,7 +29,7 @@ pub struct MusicHoard<Database, Library> {
database: Database, database: Database,
database_cache: Collection, database_cache: Collection,
library: Library, library: Library,
library_cache: HashMap<ArtistId, Artist>, library_cache: Collection,
} }
/// Phantom type for when a library implementation is not needed. /// Phantom type for when a library implementation is not needed.