From 563782f82d61fdc576d1948dcf25e52509eabfd0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 31 Aug 2024 22:38:53 +0200 Subject: [PATCH] Clarify item vs meta operations --- src/core/collection/album.rs | 46 ++++------ src/core/collection/artist.rs | 146 ++++++++++++++++---------------- src/core/musichoard/database.rs | 26 +++--- src/tui/app/selection/album.rs | 5 +- src/tui/app/selection/artist.rs | 5 +- 5 files changed, 111 insertions(+), 117 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index cc77cf8..2ca28b6 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -150,29 +150,9 @@ impl Album { } } - pub fn get_sort_key(&self) -> (&AlbumDate, &AlbumSeq, &AlbumId) { - self.meta.get_sort_key() - } - pub fn get_status(&self) -> AlbumStatus { AlbumStatus::from_tracks(&self.tracks) } - - pub fn set_seq(&mut self, seq: AlbumSeq) { - self.meta.set_seq(seq); - } - - pub fn clear_seq(&mut self) { - self.meta.clear_seq(); - } - - pub fn set_musicbrainz_ref(&mut self, mbref: MbAlbumRef) { - self.meta.set_musicbrainz_ref(mbref); - } - - pub fn clear_musicbrainz_ref(&mut self) { - self.meta.clear_musicbrainz_ref(); - } } impl PartialOrd for Album { @@ -306,13 +286,13 @@ mod tests { title: String::from("album z"), }; let mut album_1 = Album::new(album_id_1, date.clone(), None, vec![]); - album_1.set_seq(AlbumSeq(1)); + album_1.meta.set_seq(AlbumSeq(1)); let album_id_2 = AlbumId { title: String::from("album a"), }; let mut album_2 = Album::new(album_id_2, date.clone(), None, vec![]); - album_2.set_seq(AlbumSeq(2)); + album_2.meta.set_seq(AlbumSeq(2)); assert_ne!(album_1, album_2); assert!(album_1 < album_2); @@ -326,17 +306,17 @@ mod tests { assert_eq!(album.meta.seq, AlbumSeq(0)); // Setting a seq on an album. - album.set_seq(AlbumSeq(6)); + album.meta.set_seq(AlbumSeq(6)); assert_eq!(album.meta.seq, AlbumSeq(6)); - album.set_seq(AlbumSeq(6)); + album.meta.set_seq(AlbumSeq(6)); assert_eq!(album.meta.seq, AlbumSeq(6)); - album.set_seq(AlbumSeq(8)); + album.meta.set_seq(AlbumSeq(8)); assert_eq!(album.meta.seq, AlbumSeq(8)); // Clearing seq. - album.clear_seq(); + album.meta.clear_seq(); assert_eq!(album.meta.seq, AlbumSeq(0)); } @@ -388,19 +368,25 @@ mod tests { assert_eq!(album.meta.musicbrainz, expected); // Setting a URL on an album. - album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); + album + .meta + .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); _ = expected.insert(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(album.meta.musicbrainz, expected); - album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); + album + .meta + .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(album.meta.musicbrainz, expected); - album.set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); + album + .meta + .set_musicbrainz_ref(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); _ = expected.insert(MbAlbumRef::from_url_str(MUSICBRAINZ_2).unwrap()); assert_eq!(album.meta.musicbrainz, expected); // Clearing URLs. - album.clear_musicbrainz_ref(); + album.meta.clear_musicbrainz_ref(); _ = expected.take(); assert_eq!(album.meta.musicbrainz, expected); } diff --git a/src/core/collection/artist.rs b/src/core/collection/artist.rs index 697a00a..93e4a1a 100644 --- a/src/core/collection/artist.rs +++ b/src/core/collection/artist.rs @@ -48,46 +48,6 @@ impl Artist { albums: vec![], } } - - pub fn get_sort_key(&self) -> (&ArtistId,) { - self.meta.get_sort_key() - } - - pub fn set_sort_key>(&mut self, sort: SORT) { - self.meta.set_sort_key(sort); - } - - pub fn clear_sort_key(&mut self) { - self.meta.clear_sort_key(); - } - - pub fn set_musicbrainz_ref(&mut self, mbref: MbArtistRef) { - self.meta.set_musicbrainz_ref(mbref); - } - - pub fn clear_musicbrainz_ref(&mut self) { - self.meta.clear_musicbrainz_ref(); - } - - // 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 - - pub fn add_to_property + Into>(&mut self, property: S, values: Vec) { - self.meta.add_to_property(property, values); - } - - pub fn remove_from_property>(&mut self, property: S, values: Vec) { - self.meta.remove_from_property(property, values); - } - - pub fn set_property + Into>(&mut self, property: S, values: Vec) { - self.meta.set_property(property, values); - } - - pub fn clear_property>(&mut self, property: S) { - self.meta.clear_property(property); - } } impl PartialOrd for Artist { @@ -254,37 +214,41 @@ mod tests { assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort, None); - assert_eq!(artist.get_sort_key(), (&artist_id,)); - assert!(artist < Artist::new(sort_id_1.clone())); - assert!(artist < Artist::new(sort_id_2.clone())); + assert_eq!(artist.meta.get_sort_key(), (&artist_id,)); assert!(artist.meta < ArtistMeta::new(sort_id_1.clone())); assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); + assert!(artist < Artist::new(sort_id_1.clone())); + assert!(artist < Artist::new(sort_id_2.clone())); - artist.set_sort_key(sort_id_1.clone()); + artist.meta.set_sort_key(sort_id_1.clone()); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_1)); - assert_eq!(artist.get_sort_key(), (&sort_id_1,)); - assert!(artist > Artist::new(artist_id.clone())); - assert!(artist < Artist::new(sort_id_2.clone())); + assert_eq!(artist.meta.get_sort_key(), (&sort_id_1,)); assert!(artist.meta > ArtistMeta::new(artist_id.clone())); assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); + assert!(artist > Artist::new(artist_id.clone())); + assert!(artist < Artist::new(sort_id_2.clone())); - artist.set_sort_key(sort_id_2.clone()); + artist.meta.set_sort_key(sort_id_2.clone()); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort.as_ref(), Some(&sort_id_2)); - assert_eq!(artist.get_sort_key(), (&sort_id_2,)); + assert_eq!(artist.meta.get_sort_key(), (&sort_id_2,)); assert!(artist.meta > ArtistMeta::new(artist_id.clone())); assert!(artist.meta > ArtistMeta::new(sort_id_1.clone())); + assert!(artist > Artist::new(artist_id.clone())); + assert!(artist > Artist::new(sort_id_1.clone())); - artist.clear_sort_key(); + artist.meta.clear_sort_key(); assert_eq!(artist.meta.id, artist_id); assert_eq!(artist.meta.sort, None); - assert_eq!(artist.get_sort_key(), (&artist_id,)); + assert_eq!(artist.meta.get_sort_key(), (&artist_id,)); assert!(artist.meta < ArtistMeta::new(sort_id_1.clone())); assert!(artist.meta < ArtistMeta::new(sort_id_2.clone())); + assert!(artist < Artist::new(sort_id_1.clone())); + assert!(artist < Artist::new(sort_id_2.clone())); } #[test] @@ -295,19 +259,25 @@ mod tests { assert_eq!(artist.meta.musicbrainz, expected); // Setting a URL on an artist. - artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); + artist + .meta + .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); _ = expected.insert(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); - artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); + artist + .meta + .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); - artist.set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); + artist + .meta + .set_musicbrainz_ref(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); _ = expected.insert(MbArtistRef::from_url_str(MUSICBRAINZ_2).unwrap()); assert_eq!(artist.meta.musicbrainz, expected); // Clearing URLs. - artist.clear_musicbrainz_ref(); + artist.meta.clear_musicbrainz_ref(); _ = expected.take(); assert_eq!(artist.meta.musicbrainz, expected); } @@ -320,65 +290,93 @@ mod tests { assert!(artist.meta.properties.is_empty()); // Adding a single URL. - artist.add_to_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Adding a URL that already exists is ok, but does not do anything. - artist.add_to_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER]); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Adding another single URL. - artist.add_to_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER_2]); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.add_to_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER_2]); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing a URL. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER]); expected.retain(|url| url != MUSICBUTLER); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing URls that do not exist is okay, they will be ignored. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER]); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing a URL. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER_2]); expected.retain(|url| url.as_str() != MUSICBUTLER_2); assert!(artist.meta.properties.is_empty()); - artist.remove_from_property("MusicButler", vec![MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER_2]); assert!(artist.meta.properties.is_empty()); // Adding URLs if some exist is okay, they will be ignored. - artist.add_to_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing URLs if some do not exist is okay, they will be ignored. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER]); expected.retain(|url| url.as_str() != MUSICBUTLER); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.retain(|url| url.as_str() != MUSICBUTLER_2); assert!(artist.meta.properties.is_empty()); // Adding mutliple URLs without clashes. - artist.add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .add_to_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Removing multiple URLs without clashes. - artist.remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .remove_from_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.clear(); assert!(artist.meta.properties.is_empty()); } @@ -391,23 +389,25 @@ mod tests { assert!(artist.meta.properties.is_empty()); // Set URLs. - artist.set_property("MusicButler", vec![MUSICBUTLER]); + artist.meta.set_property("MusicButler", vec![MUSICBUTLER]); expected.push(MUSICBUTLER.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.set_property("MusicButler", vec![MUSICBUTLER_2]); + artist.meta.set_property("MusicButler", vec![MUSICBUTLER_2]); expected.clear(); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); - artist.set_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); + artist + .meta + .set_property("MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2]); expected.clear(); expected.push(MUSICBUTLER.to_owned()); expected.push(MUSICBUTLER_2.to_owned()); assert_eq!(artist.meta.properties.get("MusicButler"), Some(&expected)); // Clear URLs. - artist.clear_property("MusicButler"); + artist.meta.clear_property("MusicButler"); expected.clear(); assert!(artist.meta.properties.is_empty()); } diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 9926966..25287ac 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -107,7 +107,7 @@ impl IMusicHoardDatabase for MusicHoard Result<(), Error> { self.update_artist_and( artist_id.as_ref(), - |artist| artist.set_sort_key(artist_sort), + |artist| artist.meta.set_sort_key(artist_sort), |collection| Self::sort_artists(collection), ) } @@ -115,7 +115,7 @@ impl IMusicHoardDatabase for MusicHoard>(&mut self, artist_id: Id) -> Result<(), Error> { self.update_artist_and( artist_id.as_ref(), - |artist| artist.clear_sort_key(), + |artist| artist.meta.clear_sort_key(), |collection| Self::sort_artists(collection), ) } @@ -126,14 +126,18 @@ impl IMusicHoardDatabase for MusicHoard Result<(), Error> { let mb = MbArtistRef::from_url_str(url)?; - self.update_artist(artist_id.as_ref(), |artist| artist.set_musicbrainz_ref(mb)) + self.update_artist(artist_id.as_ref(), |artist| { + artist.meta.set_musicbrainz_ref(mb) + }) } fn clear_artist_musicbrainz>( &mut self, artist_id: Id, ) -> Result<(), Error> { - self.update_artist(artist_id.as_ref(), |artist| artist.clear_musicbrainz_ref()) + self.update_artist(artist_id.as_ref(), |artist| { + artist.meta.clear_musicbrainz_ref() + }) } fn add_to_artist_property, S: AsRef + Into>( @@ -143,7 +147,7 @@ impl IMusicHoardDatabase for MusicHoard, ) -> Result<(), Error> { self.update_artist(artist_id.as_ref(), |artist| { - artist.add_to_property(property, values) + artist.meta.add_to_property(property, values) }) } @@ -154,7 +158,7 @@ impl IMusicHoardDatabase for MusicHoard, ) -> Result<(), Error> { self.update_artist(artist_id.as_ref(), |artist| { - artist.remove_from_property(property, values) + artist.meta.remove_from_property(property, values) }) } @@ -165,7 +169,7 @@ impl IMusicHoardDatabase for MusicHoard, ) -> Result<(), Error> { self.update_artist(artist_id.as_ref(), |artist| { - artist.set_property(property, values) + artist.meta.set_property(property, values) }) } @@ -174,7 +178,9 @@ impl IMusicHoardDatabase for MusicHoard Result<(), Error> { - self.update_artist(artist_id.as_ref(), |artist| artist.clear_property(property)) + self.update_artist(artist_id.as_ref(), |artist| { + artist.meta.clear_property(property) + }) } fn set_album_seq, AlbumIdRef: AsRef>( @@ -186,7 +192,7 @@ impl IMusicHoardDatabase for MusicHoard IMusicHoardDatabase for MusicHoard) { if let Some(album) = album { - let result = albums.binary_search_by(|a| a.get_sort_key().cmp(&album.get_sort_key())); + let result = + albums.binary_search_by(|a| a.meta.get_sort_key().cmp(&album.get_sort_key())); match result { Ok(index) => self.reinitialise_with_index(albums, index, album.track), Err(index) => self.reinitialise_with_index(albums, index, None), @@ -169,7 +170,7 @@ impl KeySelectAlbum { pub fn get(albums: &[Album], selection: &AlbumSelection) -> Option { selection.state.list.selected().map(|index| { let album = &albums[index]; - let key = album.get_sort_key(); + let key = album.meta.get_sort_key(); KeySelectAlbum { key: (key.0.to_owned(), key.1.to_owned(), key.2.to_owned()), track: KeySelectTrack::get(&album.tracks, &selection.track), diff --git a/src/tui/app/selection/artist.rs b/src/tui/app/selection/artist.rs index aa96d25..55877b7 100644 --- a/src/tui/app/selection/artist.rs +++ b/src/tui/app/selection/artist.rs @@ -29,7 +29,8 @@ impl ArtistSelection { pub fn reinitialise(&mut self, artists: &[Artist], active: Option) { if let Some(active) = active { - let result = artists.binary_search_by(|a| a.get_sort_key().cmp(&active.get_sort_key())); + let result = + artists.binary_search_by(|a| a.meta.get_sort_key().cmp(&active.get_sort_key())); match result { Ok(index) => self.reinitialise_with_index(artists, index, active.album), Err(index) => self.reinitialise_with_index(artists, index, None), @@ -202,7 +203,7 @@ impl KeySelectArtist { pub fn get(artists: &[Artist], selection: &ArtistSelection) -> Option { selection.state.list.selected().map(|index| { let artist = &artists[index]; - let key = artist.get_sort_key(); + let key = artist.meta.get_sort_key(); KeySelectArtist { key: (key.0.to_owned(),), album: KeySelectAlbum::get(&artist.albums, &selection.album),