wojtek/musichoard
1
0
Fork 0
Code Issues 12 Pull Requests Actions Packages Projects 1 Releases 2 Wiki Activity

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
Conversation 2 Commits 8 Files Changed 2 +57 -51
1 changed files with 57 additions and 51 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 639e464ff5 - Show all commits

108
src/lib.rs
View File

@ -5,7 +5,7 @@ pub mod library;
use std::{ use std::{
cmp::Ordering, cmp::Ordering,
collections::{HashMap, HashSet}, collections::HashMap,
fmt::{self, Debug, Display}, fmt::{self, Debug, Display},
iter::Peekable, iter::Peekable,
mem, mem,
@ -856,15 +856,15 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
fn sort(collection: &mut [Artist]) { fn sort(collection: &mut [Artist]) {
Self::sort_artists(collection); Self::sort_artists(collection);
Self::sort_albums_and_tracks(collection); Self::sort_albums_and_tracks(collection.iter_mut());
} }
fn sort_artists(collection: &mut [Artist]) { fn sort_artists(collection: &mut [Artist]) {
collection.sort_unstable(); collection.sort_unstable();
} }
fn sort_albums_and_tracks(collection: &mut [Artist]) { fn sort_albums_and_tracks<'a, COL: Iterator<Item = &'a mut Artist>>(collection: COL) {
for artist in collection.iter_mut() { for artist in collection {
artist.albums.sort_unstable(); artist.albums.sort_unstable();
for album in artist.albums.iter_mut() { for album in artist.albums.iter_mut() {
album.tracks.sort_unstable(); album.tracks.sort_unstable();
@ -872,11 +872,9 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
} }
} }
fn merge_with_primary(&mut self, primary: Vec<Artist>) { fn merge_with_primary(&mut self, primary: HashMap<ArtistId, Artist>) {
let primary_map: HashMap<ArtistId, Artist> =
primary.into_iter().map(|a| (a.id.clone(), a)).collect();
let collection = mem::take(&mut self.collection); let collection = mem::take(&mut self.collection);
self.collection = Self::merge_collections(primary_map, collection); self.collection = Self::merge_collections(primary, collection);
} }
fn merge_with_secondary<SEC: IntoIterator<Item = Artist>>(&mut self, secondary: SEC) { fn merge_with_secondary<SEC: IntoIterator<Item = Artist>>(&mut self, secondary: SEC) {
@ -904,9 +902,8 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
collection collection
} }
fn items_to_artists(items: Vec<Item>) -> Result<Vec<Artist>, Error> { fn items_to_artists(items: Vec<Item>) -> Result<HashMap<ArtistId, Artist>, Error> {
let mut artists: Vec<Artist> = vec![]; let mut collection = HashMap::<ArtistId, Artist>::new();
let mut album_ids = HashMap::<ArtistId, HashSet<AlbumId>>::new();
for item in items.into_iter() { for item in items.into_iter() {
let artist_id = ArtistId { let artist_id = ArtistId {
@ -932,19 +929,14 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
}, },
}; };
let artist = if album_ids.contains_key(&artist_id) { // There are usually many entries per artist. Therefore, we avoid simply calling
// Assume results are in some order which means they will likely be grouped by // .entry(artist_id.clone()).or_insert_with(..), because of the clone. The flipside is
// artist. Therefore, we look from the back since the last inserted artist is most // that insertions will thus do an additional lookup.
// likely the one we are looking for. let artist = match collection.get_mut(&artist_id) {
artists Some(artist) => artist,
.iter_mut() None => collection
.rev() .entry(artist_id.clone())
.find(|a| a.id == artist_id) .or_insert_with(|| Artist::new(artist_id)),
.unwrap()
} else {
album_ids.insert(artist_id.clone(), HashSet::<AlbumId>::new());
artists.push(Artist::new(artist_id.clone()));
artists.last_mut().unwrap()
}; };
if artist.sort.is_some() { if artist.sort.is_some() {
@ -960,30 +952,23 @@ impl<LIB, DB> MusicHoard<LIB, DB> {
artist.sort = artist_sort; artist.sort = artist_sort;
} }
if album_ids[&artist_id].contains(&album_id) { // Do a linear search as few artists have more than a handful of albums. Search from the
// Assume results are in some order which means they will likely be grouped by // back as the original items vector is usually already sorted.
// album. Therefore, we look from the back since the last inserted album is most match artist
// likely the one we are looking for. .albums
let album = artist .iter_mut()
.albums .rev()
.iter_mut() .find(|album| album.id == album_id)
.rev() {
.find(|a| a.id == album_id) Some(album) => album.tracks.push(track),
.unwrap(); None => artist.albums.push(Album {
album.tracks.push(track);
} else {
album_ids
.get_mut(&artist_id)
.unwrap()
.insert(album_id.clone());
artist.albums.push(Album {
id: album_id, id: album_id,
tracks: vec![track], tracks: vec![track],
}); }),
} }
} }
Ok(artists) Ok(collection)
} }
fn get_artist(&self, artist_id: &ArtistId) -> Option<&Artist> { fn get_artist(&self, artist_id: &ArtistId) -> Option<&Artist> {
@ -1006,7 +991,7 @@ 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_albums_and_tracks(&mut library_collection); Self::sort_albums_and_tracks(library_collection.values_mut());
self.merge_with_primary(library_collection); self.merge_with_primary(library_collection);
Ok(()) Ok(())
@ -1017,7 +1002,7 @@ 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_albums_and_tracks(&mut database_collection); Self::sort_albums_and_tracks(database_collection.iter_mut());
self.merge_with_secondary(database_collection); self.merge_with_secondary(database_collection);
Ok(()) Ok(())
@ -1994,14 +1979,21 @@ mod tests {
expected.sort_unstable(); expected.sort_unstable();
let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections( let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
left.clone().into_iter().map(|a| (a.id.clone(), a)).collect(), left.clone()
.into_iter()
.map(|a| (a.id.clone(), a))
.collect(),
right.clone(), right.clone(),
); );
assert_eq!(expected, merged); assert_eq!(expected, merged);
// The merge is completele non-overlapping so it should be commutative. // The merge is completele non-overlapping so it should be commutative.
let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections( let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
right.clone().into_iter().map(|a| (a.id.clone(), a)).collect(), right
.clone()
.into_iter()
.map(|a| (a.id.clone(), a))
.collect(),
left.clone(), left.clone(),
); );
assert_eq!(expected, merged); assert_eq!(expected, merged);
@ -2018,14 +2010,21 @@ mod tests {
expected.sort_unstable(); expected.sort_unstable();
let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections( let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
left.clone().into_iter().map(|a| (a.id.clone(), a)).collect(), left.clone()
.into_iter()
.map(|a| (a.id.clone(), a))
.collect(),
right.clone(), right.clone(),
); );
assert_eq!(expected, merged); assert_eq!(expected, merged);
// 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 merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections( let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
right.clone().into_iter().map(|a| (a.id.clone(), a)).collect(), right
.clone()
.into_iter()
.map(|a| (a.id.clone(), a))
.collect(),
left.clone(), left.clone(),
); );
assert_eq!(expected, merged); assert_eq!(expected, merged);
@ -2055,7 +2054,10 @@ mod tests {
expected.rotate_right(1); expected.rotate_right(1);
let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections( let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
left.clone().into_iter().map(|a| (a.id.clone(), a)).collect(), left.clone()
.into_iter()
.map(|a| (a.id.clone(), a))
.collect(),
right.clone(), right.clone(),
); );
assert_eq!(expected.len(), merged.len()); assert_eq!(expected.len(), merged.len());
@ -2063,7 +2065,11 @@ 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 merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections( let merged = MusicHoard::<NoLibrary, NoDatabase>::merge_collections(
right.clone().into_iter().map(|a| (a.id.clone(), a)).collect(), right
.clone()
.into_iter()
.map(|a| (a.id.clone(), a))
.collect(),
left.clone(), left.clone(),
); );
assert_eq!(expected.len(), merged.len()); assert_eq!(expected.len(), merged.len());