Artists with a _sort field show up twice #109

Merged
wojtek merged 8 commits from 108---artists-with-a-_sort-field-show-up-twice into main 2024-01-14 15:46:34 +01:00
Showing only changes of commit 56e4329eca - Show all commits

View File

@ -287,9 +287,8 @@ impl Ord for Track {
} }
impl Merge for Track { impl Merge for Track {
fn merge(self, other: Self) -> Self { fn merge_in_place(&mut self, other: Self) {
assert_eq!(self.id, other.id); assert_eq!(self.id, other.id);
self
} }
} }
@ -320,10 +319,10 @@ impl Ord for Album {
} }
impl Merge for Album { impl Merge for Album {
fn merge(mut self, other: Self) -> Self { fn merge_in_place(&mut self, other: Self) {
assert_eq!(self.id, other.id); assert_eq!(self.id, other.id);
self.tracks = MergeSorted::new(self.tracks.into_iter(), other.tracks.into_iter()).collect(); let tracks = mem::take(&mut self.tracks);
self self.tracks = MergeSorted::new(tracks.into_iter(), other.tracks.into_iter()).collect();
} }
} }
@ -361,13 +360,11 @@ pub struct ArtistProperties {
} }
impl Merge for ArtistProperties { impl Merge for ArtistProperties {
fn merge(mut self, other: Self) -> Self { fn merge_in_place(&mut self, other: Self) {
self.musicbrainz = Self::merge_opts(self.musicbrainz, other.musicbrainz); self.musicbrainz = self.musicbrainz.take().or(other.musicbrainz);
self.musicbutler = Self::merge_vecs(self.musicbutler, other.musicbutler); Self::merge_vecs(&mut self.musicbutler, other.musicbutler);
self.bandcamp = Self::merge_vecs(self.bandcamp, other.bandcamp); Self::merge_vecs(&mut self.bandcamp, other.bandcamp);
self.qobuz = Self::merge_opts(self.qobuz, other.qobuz); self.qobuz = self.qobuz.take().or(other.qobuz);
self
} }
} }
@ -566,12 +563,12 @@ impl Ord for Artist {
} }
impl Merge for Artist { impl Merge for Artist {
fn merge(mut self, other: Self) -> Self { fn merge_in_place(&mut self, other: Self) {
assert_eq!(self.id, other.id); assert_eq!(self.id, other.id);
self.sort = Self::merge_opts(self.sort, other.sort); self.sort = self.sort.take().or(other.sort);
self.properties = self.properties.merge(other.properties); self.properties.merge_in_place(other.properties);
self.albums = MergeSorted::new(self.albums.into_iter(), other.albums.into_iter()).collect(); let albums = mem::take(&mut self.albums);
self self.albums = MergeSorted::new(albums.into_iter(), other.albums.into_iter()).collect();
} }
} }
@ -579,20 +576,20 @@ impl Merge for Artist {
pub type Collection = Vec<Artist>; pub type Collection = Vec<Artist>;
trait Merge { trait Merge {
fn merge(self, other: Self) -> Self; fn merge_in_place(&mut self, other: Self);
fn merge_opts<T>(this: Option<T>, other: Option<T>) -> Option<T> { fn merge(mut self, other: Self) -> Self
match &this { where
Some(_) => this, Self: Sized,
None => other, {
} self.merge_in_place(other);
self
} }
fn merge_vecs<T: Ord + Eq>(mut this: Vec<T>, mut other: Vec<T>) -> Vec<T> { fn merge_vecs<T: Ord + Eq>(this: &mut Vec<T>, mut other: Vec<T>) {
this.append(&mut other); this.append(&mut other);
this.sort_unstable(); this.sort_unstable();
this.dedup(); this.dedup();
this
} }
} }
@ -816,9 +813,7 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
if self.get_artist(&artist_id).is_none() { if self.get_artist(&artist_id).is_none() {
let new_artist = vec![Artist::new(artist_id)]; let new_artist = vec![Artist::new(artist_id)];
Self::merge_in_place(&mut self.collection, new_artist);
let collection = mem::take(&mut self.collection);
self.collection = Self::merge(collection, new_artist);
} }
} }
@ -860,7 +855,15 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
music_hoard_unique_url_dispatch!(qobuz); music_hoard_unique_url_dispatch!(qobuz);
fn sort(collection: &mut [Artist]) { fn sort(collection: &mut [Artist]) {
Self::sort_artists(collection);
Self::sort_albums_and_tracks(collection);
}
fn sort_artists(collection: &mut [Artist]) {
collection.sort_unstable(); collection.sort_unstable();
}
fn sort_albums_and_tracks(collection: &mut [Artist]) {
for artist in collection.iter_mut() { for artist in collection.iter_mut() {
artist.albums.sort_unstable(); artist.albums.sort_unstable();
for album in artist.albums.iter_mut() { for album in artist.albums.iter_mut() {
@ -869,8 +872,18 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
} }
} }
fn merge(primary: Vec<Artist>, secondary: Vec<Artist>) -> Vec<Artist> { fn merge_in_place(primary: &mut Vec<Artist>, secondary: Vec<Artist>) {
MergeSorted::new(primary.into_iter(), secondary.into_iter()).collect() let mut primary_map: HashMap<ArtistId, Artist> =
primary.drain(..).map(|a| (a.id.clone(), a)).collect();
for secondary_artist in secondary.into_iter() {
if let Some(ref mut primary_artist) = primary_map.get_mut(&secondary_artist.id) {
primary_artist.merge_in_place(secondary_artist);
} else {
primary_map.insert(secondary_artist.id.clone(), secondary_artist);
}
}
primary.extend(primary_map.into_values());
Self::sort_artists(primary);
} }
fn items_to_artists(items: Vec<Item>) -> Result<Vec<Artist>, Error> { fn items_to_artists(items: Vec<Item>) -> Result<Vec<Artist>, Error> {
@ -975,10 +988,10 @@ impl<LIB: ILibrary, DB> MusicHoard<LIB, DB> {
pub fn rescan_library(&mut self) -> Result<(), Error> { pub fn rescan_library(&mut self) -> Result<(), Error> {
let items = self.library.list(&Query::new())?; let items = self.library.list(&Query::new())?;
let mut library_collection = Self::items_to_artists(items)?; let mut library_collection = Self::items_to_artists(items)?;
Self::sort(&mut library_collection); Self::sort_albums_and_tracks(&mut library_collection);
let collection = mem::take(&mut self.collection); Self::merge_in_place(&mut library_collection, mem::take(&mut self.collection));
self.collection = Self::merge(library_collection, collection); mem::swap(&mut library_collection, &mut self.collection);
Ok(()) Ok(())
} }
@ -988,10 +1001,9 @@ impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
/// Load the database and merge with the in-memory collection. /// Load the database and merge with the in-memory collection.
pub fn load_from_database(&mut self) -> Result<(), Error> { pub fn load_from_database(&mut self) -> Result<(), Error> {
let mut database_collection = self.database.load()?; let mut database_collection = self.database.load()?;
Self::sort(&mut database_collection); Self::sort_albums_and_tracks(&mut database_collection);
let collection = mem::take(&mut self.collection); Self::merge_in_place(&mut self.collection, database_collection);
self.collection = Self::merge(collection, database_collection);
Ok(()) Ok(())
} }
@ -1957,7 +1969,8 @@ mod tests {
let mut expected = COLLECTION.to_owned(); let mut expected = COLLECTION.to_owned();
expected.sort_unstable(); expected.sort_unstable();
let merged = MusicHoard::<NoLibrary, NoDatabase>::merge(left.clone(), right); let mut merged = left;
MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, right);
assert_eq!(expected, merged); assert_eq!(expected, merged);
} }
@ -1971,7 +1984,8 @@ mod tests {
let mut expected = COLLECTION.to_owned(); let mut expected = COLLECTION.to_owned();
expected.sort_unstable(); expected.sort_unstable();
let merged = MusicHoard::<NoLibrary, NoDatabase>::merge(left.clone(), right); let mut merged = left;
MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, right);
assert_eq!(expected, merged); assert_eq!(expected, merged);
} }
@ -1998,7 +2012,8 @@ mod tests {
expected.last_mut().map(|a| a.sort = artist_sort.clone()); expected.last_mut().map(|a| a.sort = artist_sort.clone());
expected.rotate_right(1); expected.rotate_right(1);
let merged = MusicHoard::<NoLibrary, NoDatabase>::merge(left.clone(), right); let mut merged = left;
MusicHoard::<NoLibrary, NoDatabase>::merge_in_place(&mut merged, right);
assert_eq!(expected.len(), merged.len()); assert_eq!(expected.len(), merged.len());
assert_eq!(expected, merged); assert_eq!(expected, merged);
} }