From 857c07d599c7eabd32d149e0c7a436d83435cf97 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 15:40:25 +0100 Subject: [PATCH 01/14] Add fields for filtering --- src/core/collection/album.rs | 1 + src/core/musichoard/builder.rs | 10 +++++++- src/core/musichoard/mod.rs | 43 ++++++++++++++++++++++++++++++---- 3 files changed, 48 insertions(+), 6 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index 308e901..95dbefb 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -138,6 +138,7 @@ pub enum AlbumSecondaryType { } /// The album's ownership status. +#[derive(Debug)] pub enum AlbumOwnership { None, Owned(TrackFormat), diff --git a/src/core/musichoard/builder.rs b/src/core/musichoard/builder.rs index 5ea72ce..a68aac9 100644 --- a/src/core/musichoard/builder.rs +++ b/src/core/musichoard/builder.rs @@ -1,6 +1,6 @@ use crate::core::{ interface::{database::IDatabase, library::ILibrary}, - musichoard::{database::IMusicHoardDatabase, Error, MusicHoard, NoDatabase, NoLibrary}, + musichoard::{database::IMusicHoardDatabase, Error, MusicHoard, NoDatabase, NoLibrary, CollectionFilter}, }; /// Builder for [`MusicHoard`]. Its purpose is to make it easier to set various combinations of @@ -62,6 +62,8 @@ impl MusicHoard { /// Create a new [`MusicHoard`] without any library or database. pub fn empty() -> Self { MusicHoard { + filter: CollectionFilter::default(), + filtered: vec![], collection: vec![], pre_commit: vec![], database: NoDatabase, @@ -83,6 +85,8 @@ impl MusicHoard { /// Create a new [`MusicHoard`] with the provided [`ILibrary`] and no database. pub fn library(library: Library) -> Self { MusicHoard { + filter: CollectionFilter::default(), + filtered: vec![], collection: vec![], pre_commit: vec![], database: NoDatabase, @@ -104,6 +108,8 @@ impl MusicHoard { /// Create a new [`MusicHoard`] with the provided [`IDatabase`] and no library. pub fn database(database: Database) -> Result { let mut mh = MusicHoard { + filter: CollectionFilter::default(), + filtered: vec![], collection: vec![], pre_commit: vec![], database, @@ -127,6 +133,8 @@ impl MusicHoard { /// Create a new [`MusicHoard`] with the provided [`ILibrary`] and [`IDatabase`]. pub fn new(database: Database, library: Library) -> Result { let mut mh = MusicHoard { + filter: CollectionFilter::default(), + filtered: vec![], collection: vec![], pre_commit: vec![], database, diff --git a/src/core/musichoard/mod.rs b/src/core/musichoard/mod.rs index f6ba502..8fb7026 100644 --- a/src/core/musichoard/mod.rs +++ b/src/core/musichoard/mod.rs @@ -12,11 +12,16 @@ pub use library::IMusicHoardLibrary; use std::fmt::{self, Display}; -use crate::core::collection::Collection; - -use crate::core::interface::{ - database::{LoadError as DatabaseLoadError, SaveError as DatabaseSaveError}, - library::Error as LibraryError, +use crate::core::{ + collection::{ + album::{AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}, + track::TrackFormat, + Collection, + }, + interface::{ + database::{LoadError as DatabaseLoadError, SaveError as DatabaseSaveError}, + library::Error as LibraryError, + }, }; /// The Music Hoard. It is responsible for pulling information from both the library and the @@ -24,6 +29,8 @@ use crate::core::interface::{ // TODO: Split into inner and external/interfaces to facilitate building. #[derive(Debug)] pub struct MusicHoard { + filter: CollectionFilter, + filtered: Collection, collection: Collection, pre_commit: Collection, database: Database, @@ -32,6 +39,32 @@ pub struct MusicHoard { library_cache: Collection, } +/// Filter for a specifying subsets of the entire collection (e.g., for display). +// Filter is inclusive, not exclusive. To include something in the filtered collection, the filter +// must select it. It also means that the options below are additive - the more options are set, the +// more items will be included. +#[derive(Debug)] +pub struct CollectionFilter { + primary_types: Vec, + secondary_types: Vec, + include_untyped: bool, + ownership: Vec, +} + +impl Default for CollectionFilter { + fn default() -> Self { + CollectionFilter { + primary_types: vec![AlbumPrimaryType::Ep, AlbumPrimaryType::Album], + secondary_types: vec![], + include_untyped: true, + ownership: vec![ + AlbumOwnership::Owned(TrackFormat::Mp3), + AlbumOwnership::Owned(TrackFormat::Flac), + ], + } + } +} + /// Phantom type for when a library implementation is not needed. #[derive(Debug)] pub struct NoLibrary; -- 2.47.1 From eac27d3b17dd31fca2c964c65e65b9d4054178c1 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 17:26:09 +0100 Subject: [PATCH 02/14] Code draft --- src/core/collection/album.rs | 2 +- src/core/musichoard/base.rs | 33 ++++++++++++++++++++++- src/core/musichoard/filter.rs | 50 +++++++++++++++++++++++++++++++++++ src/core/musichoard/mod.rs | 34 +++--------------------- src/lib.rs | 4 +-- src/main.rs | 38 +++++++++++++++++++++----- 6 files changed, 120 insertions(+), 41 deletions(-) create mode 100644 src/core/musichoard/filter.rs diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index 95dbefb..e5d0033 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -138,7 +138,7 @@ pub enum AlbumSecondaryType { } /// The album's ownership status. -#[derive(Debug)] +#[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum AlbumOwnership { None, Owned(TrackFormat), diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index f247eb0..db8f910 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -5,14 +5,25 @@ use crate::core::{ merge::{MergeCollections, NormalMap}, string, Collection, }, - musichoard::{Error, MusicHoard}, + musichoard::{filter::CollectionFilter, Error, MusicHoard}, }; pub trait IMusicHoardBase { + fn set_filter(&mut self, filter: CollectionFilter); + fn get_filtered(&self) -> &Collection; fn get_collection(&self) -> &Collection; } impl IMusicHoardBase for MusicHoard { + fn set_filter(&mut self, filter: CollectionFilter) { + self.filter = filter; + self.filtered = self.filter_collection(); + } + + fn get_filtered(&self) -> &Collection { + &self.filtered + } + fn get_collection(&self) -> &Collection { &self.collection } @@ -23,6 +34,8 @@ pub trait IMusicHoardBasePrivate { fn sort_albums_and_tracks<'a, C: Iterator>(collection: C); fn merge_collections(&self) -> Collection; + fn filter_collection(&self) -> Collection; + fn filter_artist(&self, artist: &Artist) -> Option; fn get_artist<'a>(collection: &'a Collection, artist_id: &ArtistId) -> Option<&'a Artist>; fn get_artist_mut<'a>( @@ -74,6 +87,24 @@ impl IMusicHoardBasePrivate for MusicHoard collection } + fn filter_collection(&self) -> Collection { + let iter = self.collection.iter(); + iter.map(|a| self.filter_artist(a)).flatten().collect() + } + + fn filter_artist(&self, artist: &Artist) -> Option { + let iter = artist.albums.iter(); + let filtered = iter.filter(|a| self.filter.filter_album(a)); + let albums: Vec = filtered.cloned().collect(); + + if albums.is_empty() { + return None; + } + + let meta = artist.meta.clone(); + Some(Artist { meta, albums }) + } + fn get_artist<'a>(collection: &'a Collection, artist_id: &ArtistId) -> Option<&'a Artist> { collection.iter().find(|a| &a.meta.id == artist_id) } diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs new file mode 100644 index 0000000..d58fcb6 --- /dev/null +++ b/src/core/musichoard/filter.rs @@ -0,0 +1,50 @@ +use crate::core::collection::album::{Album, AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}; + +/// Filter for a specifying subsets of the entire collection (e.g., for display). +#[derive(Debug, Default)] +pub struct CollectionFilter { + pub include: Vec>, + pub exclude: Vec>, +} + +#[derive(Debug)] +pub enum AlbumField { + PrimaryType(Option), + SecondaryType(AlbumSecondaryType), + Ownership(AlbumOwnership), +} + +impl CollectionFilter { + pub fn filter_album(&self, album: &Album) -> bool { + let include = Self::filter_and(&self.include, album); + let exclude = Self::filter_and(&self.exclude, album); + include && !exclude + } + + fn filter_and(group: &Vec>, album: &Album) -> bool { + let mut filter = !group.is_empty(); + for group in group.iter() { + filter = filter && Self::filter_or(group, album); + } + filter + } + + fn filter_or(group: &Vec, album: &Album) -> bool { + let mut filter = false; + for field in group.iter() { + filter = filter || Self::filter_field(field, album); + } + filter + } + + fn filter_field(field: &AlbumField, album: &Album) -> bool { + match field { + AlbumField::PrimaryType(filter) => *filter == album.meta.info.primary_type, + AlbumField::SecondaryType(filter) => { + let types = &album.meta.info.secondary_types; + types.iter().find(|st| *st == filter).is_some() + } + AlbumField::Ownership(filter) => *filter == album.get_ownership(), + } + } +} diff --git a/src/core/musichoard/mod.rs b/src/core/musichoard/mod.rs index 8fb7026..3b3230d 100644 --- a/src/core/musichoard/mod.rs +++ b/src/core/musichoard/mod.rs @@ -5,19 +5,17 @@ mod database; mod library; pub mod builder; +pub mod filter; pub use base::IMusicHoardBase; pub use database::IMusicHoardDatabase; +pub use filter::CollectionFilter; pub use library::IMusicHoardLibrary; use std::fmt::{self, Display}; use crate::core::{ - collection::{ - album::{AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}, - track::TrackFormat, - Collection, - }, + collection::Collection, interface::{ database::{LoadError as DatabaseLoadError, SaveError as DatabaseSaveError}, library::Error as LibraryError, @@ -39,32 +37,6 @@ pub struct MusicHoard { library_cache: Collection, } -/// Filter for a specifying subsets of the entire collection (e.g., for display). -// Filter is inclusive, not exclusive. To include something in the filtered collection, the filter -// must select it. It also means that the options below are additive - the more options are set, the -// more items will be included. -#[derive(Debug)] -pub struct CollectionFilter { - primary_types: Vec, - secondary_types: Vec, - include_untyped: bool, - ownership: Vec, -} - -impl Default for CollectionFilter { - fn default() -> Self { - CollectionFilter { - primary_types: vec![AlbumPrimaryType::Ep, AlbumPrimaryType::Album], - secondary_types: vec![], - include_untyped: true, - ownership: vec![ - AlbumOwnership::Owned(TrackFormat::Mp3), - AlbumOwnership::Owned(TrackFormat::Flac), - ], - } - } -} - /// Phantom type for when a library implementation is not needed. #[derive(Debug)] pub struct NoLibrary; diff --git a/src/lib.rs b/src/lib.rs index 6e8d317..dd8dea7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -10,8 +10,8 @@ pub use core::collection; pub use core::interface; pub use core::musichoard::{ - builder::MusicHoardBuilder, Error, IMusicHoardBase, IMusicHoardDatabase, IMusicHoardLibrary, - MusicHoard, NoDatabase, NoLibrary, + builder::MusicHoardBuilder, filter, Error, IMusicHoardBase, IMusicHoardDatabase, + IMusicHoardLibrary, MusicHoard, NoDatabase, NoLibrary, }; #[cfg(test)] diff --git a/src/main.rs b/src/main.rs index cfbf750..bd6c6ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,19 +6,17 @@ use ratatui::{backend::CrosstermBackend, Terminal}; use structopt::StructOpt; use musichoard::{ - external::{ + collection::album::{AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}, external::{ database::json::{backend::JsonDatabaseFileBackend, JsonDatabase}, library::beets::{ executor::{ssh::BeetsLibrarySshExecutor, BeetsLibraryProcessExecutor}, BeetsLibrary, }, musicbrainz::{api::MusicBrainzClient, http::MusicBrainzHttp}, - }, - interface::{ + }, filter::{AlbumField, CollectionFilter}, interface::{ database::{IDatabase, NullDatabase}, library::{ILibrary, NullLibrary}, - }, - MusicHoardBuilder, NoDatabase, NoLibrary, + }, IMusicHoardBase, MusicHoardBuilder, NoDatabase, NoLibrary }; use tui::{ @@ -69,10 +67,38 @@ struct DbOpt { no_database: bool, } +fn default_filter() -> CollectionFilter { + CollectionFilter { + include: vec![vec![ + AlbumField::PrimaryType(None), + AlbumField::PrimaryType(Some(AlbumPrimaryType::Ep)), + AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), + ]], + exclude: vec![ + vec![AlbumField::Ownership(AlbumOwnership::None)], + vec![ + AlbumField::SecondaryType(AlbumSecondaryType::Compilation), + AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), + AlbumField::SecondaryType(AlbumSecondaryType::Spokenword), + AlbumField::SecondaryType(AlbumSecondaryType::Interview), + AlbumField::SecondaryType(AlbumSecondaryType::Audiobook), + AlbumField::SecondaryType(AlbumSecondaryType::AudioDrama), + AlbumField::SecondaryType(AlbumSecondaryType::Live), + AlbumField::SecondaryType(AlbumSecondaryType::Remix), + AlbumField::SecondaryType(AlbumSecondaryType::DjMix), + AlbumField::SecondaryType(AlbumSecondaryType::MixtapeStreet), + AlbumField::SecondaryType(AlbumSecondaryType::Demo), + AlbumField::SecondaryType(AlbumSecondaryType::FieldRecording), + ], + ], + } +} + fn with( builder: MusicHoardBuilder, ) { - let music_hoard = builder.build().expect("failed to initialise MusicHoard"); + let mut music_hoard = builder.build().expect("failed to initialise MusicHoard"); + music_hoard.set_filter(default_filter()); // Initialize the terminal user interface. let backend = CrosstermBackend::new(io::stdout()); -- 2.47.1 From 9e7f0d8092d0ab6de4c7aeba02200ee6aa585072 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 17:50:05 +0100 Subject: [PATCH 03/14] WIP --- src/core/musichoard/filter.rs | 102 +++++++++++++++++++++++++++++++++- src/main.rs | 42 +++++++------- 2 files changed, 121 insertions(+), 23 deletions(-) diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs index d58fcb6..a95c472 100644 --- a/src/core/musichoard/filter.rs +++ b/src/core/musichoard/filter.rs @@ -4,7 +4,7 @@ use crate::core::collection::album::{Album, AlbumOwnership, AlbumPrimaryType, Al #[derive(Debug, Default)] pub struct CollectionFilter { pub include: Vec>, - pub exclude: Vec>, + pub except: Vec>, } #[derive(Debug)] @@ -17,8 +17,8 @@ pub enum AlbumField { impl CollectionFilter { pub fn filter_album(&self, album: &Album) -> bool { let include = Self::filter_and(&self.include, album); - let exclude = Self::filter_and(&self.exclude, album); - include && !exclude + let except = Self::filter_and(&self.except, album); + include && !except } fn filter_and(group: &Vec>, album: &Album) -> bool { @@ -48,3 +48,99 @@ impl CollectionFilter { } } } + +#[cfg(test)] +mod tests { + use crate::collection::{ + album::AlbumId, + track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality}, + }; + + use super::*; + + fn test_filter() -> CollectionFilter { + CollectionFilter { + include: vec![vec![ + AlbumField::PrimaryType(None), + AlbumField::PrimaryType(Some(AlbumPrimaryType::Ep)), + AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), + ]], + except: vec![vec![ + AlbumField::SecondaryType(AlbumSecondaryType::Compilation), + AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), + AlbumField::SecondaryType(AlbumSecondaryType::Live), + AlbumField::SecondaryType(AlbumSecondaryType::Demo), + AlbumField::Ownership(AlbumOwnership::None), + ]], + } + } + + fn test_track() -> Track { + Track { + id: TrackId { + title: String::from("Track"), + }, + number: TrackNum(1), + artist: vec![String::from("Artist")], + quality: TrackQuality { + format: TrackFormat::Mp3, + bitrate: 320, + }, + } + } + + fn test_album() -> Album { + let mut album = Album::new(AlbumId::new("An Album")); + album.tracks.push(test_track()); + album + } + + #[test] + fn filter_primary_type() { + let filter = test_filter(); + let mut album = test_album(); + + album.meta.info.primary_type = None; + assert!(filter.filter_album(&album)); + + album.meta.info.primary_type = Some(AlbumPrimaryType::Ep); + assert!(filter.filter_album(&album)); + + album.meta.info.primary_type = Some(AlbumPrimaryType::Album); + assert!(filter.filter_album(&album)); + + album.meta.info.primary_type = Some(AlbumPrimaryType::Broadcast); + assert!(!filter.filter_album(&album)); + + album.meta.info.primary_type = Some(AlbumPrimaryType::Other); + assert!(!filter.filter_album(&album)); + } + + #[test] + fn filter_secondary_type() { + let filter = test_filter() + } + + #[test] + fn filter_ownership() { + let filter = test_filter(); + let mut album = Album::new(AlbumId::new("An Album")); + + album.tracks.clear(); + assert!(!filter.filter_album(&album)); + + album.tracks.push(test_track()); + assert_eq!( + album.get_ownership(), + AlbumOwnership::Owned(TrackFormat::Mp3) + ); + assert!(filter.filter_album(&album)); + + album.tracks[0].quality.format = TrackFormat::Flac; + assert_eq!( + album.get_ownership(), + AlbumOwnership::Owned(TrackFormat::Flac) + ); + assert!(filter.filter_album(&album)); + } +} diff --git a/src/main.rs b/src/main.rs index bd6c6ad..2e32dc5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,17 +6,21 @@ use ratatui::{backend::CrosstermBackend, Terminal}; use structopt::StructOpt; use musichoard::{ - collection::album::{AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}, external::{ + collection::album::{AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}, + external::{ database::json::{backend::JsonDatabaseFileBackend, JsonDatabase}, library::beets::{ executor::{ssh::BeetsLibrarySshExecutor, BeetsLibraryProcessExecutor}, BeetsLibrary, }, musicbrainz::{api::MusicBrainzClient, http::MusicBrainzHttp}, - }, filter::{AlbumField, CollectionFilter}, interface::{ + }, + filter::{AlbumField, CollectionFilter}, + interface::{ database::{IDatabase, NullDatabase}, library::{ILibrary, NullLibrary}, - }, IMusicHoardBase, MusicHoardBuilder, NoDatabase, NoLibrary + }, + IMusicHoardBase, MusicHoardBuilder, NoDatabase, NoLibrary, }; use tui::{ @@ -74,23 +78,21 @@ fn default_filter() -> CollectionFilter { AlbumField::PrimaryType(Some(AlbumPrimaryType::Ep)), AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), ]], - exclude: vec![ - vec![AlbumField::Ownership(AlbumOwnership::None)], - vec![ - AlbumField::SecondaryType(AlbumSecondaryType::Compilation), - AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), - AlbumField::SecondaryType(AlbumSecondaryType::Spokenword), - AlbumField::SecondaryType(AlbumSecondaryType::Interview), - AlbumField::SecondaryType(AlbumSecondaryType::Audiobook), - AlbumField::SecondaryType(AlbumSecondaryType::AudioDrama), - AlbumField::SecondaryType(AlbumSecondaryType::Live), - AlbumField::SecondaryType(AlbumSecondaryType::Remix), - AlbumField::SecondaryType(AlbumSecondaryType::DjMix), - AlbumField::SecondaryType(AlbumSecondaryType::MixtapeStreet), - AlbumField::SecondaryType(AlbumSecondaryType::Demo), - AlbumField::SecondaryType(AlbumSecondaryType::FieldRecording), - ], - ], + except: vec![vec![ + AlbumField::SecondaryType(AlbumSecondaryType::Compilation), + AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), + AlbumField::SecondaryType(AlbumSecondaryType::Spokenword), + AlbumField::SecondaryType(AlbumSecondaryType::Interview), + AlbumField::SecondaryType(AlbumSecondaryType::Audiobook), + AlbumField::SecondaryType(AlbumSecondaryType::AudioDrama), + AlbumField::SecondaryType(AlbumSecondaryType::Live), + AlbumField::SecondaryType(AlbumSecondaryType::Remix), + AlbumField::SecondaryType(AlbumSecondaryType::DjMix), + AlbumField::SecondaryType(AlbumSecondaryType::MixtapeStreet), + AlbumField::SecondaryType(AlbumSecondaryType::Demo), + AlbumField::SecondaryType(AlbumSecondaryType::FieldRecording), + AlbumField::Ownership(AlbumOwnership::None), + ]], } } -- 2.47.1 From 68d50ad986122577ae1ceea6979c429306925311 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 20:12:13 +0100 Subject: [PATCH 04/14] Working UTs --- src/core/musichoard/filter.rs | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs index a95c472..b77afd9 100644 --- a/src/core/musichoard/filter.rs +++ b/src/core/musichoard/filter.rs @@ -66,11 +66,11 @@ mod tests { AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), ]], except: vec![vec![ + AlbumField::Ownership(AlbumOwnership::None), AlbumField::SecondaryType(AlbumSecondaryType::Compilation), AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), AlbumField::SecondaryType(AlbumSecondaryType::Live), AlbumField::SecondaryType(AlbumSecondaryType::Demo), - AlbumField::Ownership(AlbumOwnership::None), ]], } } @@ -91,6 +91,7 @@ mod tests { fn test_album() -> Album { let mut album = Album::new(AlbumId::new("An Album")); + album.meta.info.primary_type = Some(AlbumPrimaryType::Album); album.tracks.push(test_track()); album } @@ -118,7 +119,29 @@ mod tests { #[test] fn filter_secondary_type() { - let filter = test_filter() + let filter = test_filter(); + let mut album = test_album(); + + assert!(filter.filter_album(&album)); + + // Non-filtered type. + let types = &mut album.meta.info.secondary_types; + types.push(AlbumSecondaryType::AudioDrama); + assert!(filter.filter_album(&album)); + + // Filtered type. + let types = &mut album.meta.info.secondary_types; + types.push(AlbumSecondaryType::Live); + assert!(!filter.filter_album(&album)); + + // Remove non-filtered type. + album.meta.info.secondary_types.remove(0); + assert!(!filter.filter_album(&album)); + + // Add another filtered type. + let types = &mut album.meta.info.secondary_types; + types.push(AlbumSecondaryType::Soundtrack); + assert!(!filter.filter_album(&album)); } #[test] -- 2.47.1 From f5644690a4bc573217a303b766d794ff41fcca68 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 20:13:22 +0100 Subject: [PATCH 05/14] Simplify filter --- src/core/musichoard/filter.rs | 24 ++++++++---------------- src/main.rs | 8 ++++---- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs index b77afd9..203a95a 100644 --- a/src/core/musichoard/filter.rs +++ b/src/core/musichoard/filter.rs @@ -3,8 +3,8 @@ use crate::core::collection::album::{Album, AlbumOwnership, AlbumPrimaryType, Al /// Filter for a specifying subsets of the entire collection (e.g., for display). #[derive(Debug, Default)] pub struct CollectionFilter { - pub include: Vec>, - pub except: Vec>, + pub include: Vec, + pub except: Vec, } #[derive(Debug)] @@ -16,19 +16,11 @@ pub enum AlbumField { impl CollectionFilter { pub fn filter_album(&self, album: &Album) -> bool { - let include = Self::filter_and(&self.include, album); - let except = Self::filter_and(&self.except, album); + let include = Self::filter_or(&self.include, album); + let except = Self::filter_or(&self.except, album); include && !except } - fn filter_and(group: &Vec>, album: &Album) -> bool { - let mut filter = !group.is_empty(); - for group in group.iter() { - filter = filter && Self::filter_or(group, album); - } - filter - } - fn filter_or(group: &Vec, album: &Album) -> bool { let mut filter = false; for field in group.iter() { @@ -60,18 +52,18 @@ mod tests { fn test_filter() -> CollectionFilter { CollectionFilter { - include: vec![vec![ + include: vec![ AlbumField::PrimaryType(None), AlbumField::PrimaryType(Some(AlbumPrimaryType::Ep)), AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), - ]], - except: vec![vec![ + ], + except: vec![ AlbumField::Ownership(AlbumOwnership::None), AlbumField::SecondaryType(AlbumSecondaryType::Compilation), AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), AlbumField::SecondaryType(AlbumSecondaryType::Live), AlbumField::SecondaryType(AlbumSecondaryType::Demo), - ]], + ], } } diff --git a/src/main.rs b/src/main.rs index 2e32dc5..760a268 100644 --- a/src/main.rs +++ b/src/main.rs @@ -73,12 +73,12 @@ struct DbOpt { fn default_filter() -> CollectionFilter { CollectionFilter { - include: vec![vec![ + include: vec![ AlbumField::PrimaryType(None), AlbumField::PrimaryType(Some(AlbumPrimaryType::Ep)), AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), - ]], - except: vec![vec![ + ], + except: vec![ AlbumField::SecondaryType(AlbumSecondaryType::Compilation), AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), AlbumField::SecondaryType(AlbumSecondaryType::Spokenword), @@ -92,7 +92,7 @@ fn default_filter() -> CollectionFilter { AlbumField::SecondaryType(AlbumSecondaryType::Demo), AlbumField::SecondaryType(AlbumSecondaryType::FieldRecording), AlbumField::Ownership(AlbumOwnership::None), - ]], + ], } } -- 2.47.1 From 925063d0e76c36c65a7311f63b4aadfa9c1d1f72 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 20:36:19 +0100 Subject: [PATCH 06/14] Broad expectation updates --- src/core/musichoard/database.rs | 4 ++++ src/tui/app/machine/browse_state.rs | 6 +++--- src/tui/app/machine/fetch_state.rs | 14 +++++++++----- src/tui/app/machine/mod.rs | 8 ++++---- src/tui/app/machine/reload_state.rs | 6 +++--- src/tui/app/machine/search_state.rs | 6 +++--- src/tui/lib/mod.rs | 6 ++++++ src/tui/mod.rs | 2 +- 8 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/core/musichoard/database.rs b/src/core/musichoard/database.rs index 0d387aa..990871c 100644 --- a/src/core/musichoard/database.rs +++ b/src/core/musichoard/database.rs @@ -120,6 +120,8 @@ impl IMusicHoardDatabase for MusicHoard IMusicHoardDatabasePrivate for MusicHoard { fn commit(&mut self) -> Result<(), Error> { self.collection = self.pre_commit.clone(); + self.filtered = self.filter_collection(); Ok(()) } } @@ -376,6 +379,7 @@ impl IMusicHoardDatabasePrivate for MusicHoard { fn increment_selection(mut self, delta: Delta) -> Self::APP { self.inner .selection - .increment_selection(self.inner.music_hoard.get_collection(), delta); + .increment_selection(self.inner.music_hoard.get_filtered(), delta); self.into() } fn decrement_selection(mut self, delta: Delta) -> Self::APP { self.inner .selection - .decrement_selection(self.inner.music_hoard.get_collection(), delta); + .decrement_selection(self.inner.music_hoard.get_filtered(), delta); self.into() } @@ -68,7 +68,7 @@ impl IAppInteractBrowse for AppMachine { let orig = ListSelection::get(&self.inner.selection); self.inner .selection - .reset(self.inner.music_hoard.get_collection()); + .reset(self.inner.music_hoard.get_filtered()); AppMachine::search_state(self.inner, orig).into() } diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 9ce871c..412c9f1 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -102,7 +102,7 @@ impl AppMachine { } fn fetch_job(inner: &AppInner, rx: MbApiReceiver) -> Result { - let coll = inner.music_hoard.get_collection(); + let coll = inner.music_hoard.get_filtered(); let artist = match inner.selection.state_artist(coll) { Some(artist_state) => &coll[artist_state.index], @@ -184,18 +184,22 @@ impl AppMachine { inner: &mut AppInner, fetch_albums: Vec, ) -> Result<(), musichoard::Error> { - let coll = inner.music_hoard.get_collection(); + let coll = inner.music_hoard.get_filtered(); let artist_state = inner.selection.state_artist(coll).unwrap(); let artist = &coll[artist_state.index]; let selection = KeySelection::get(coll, &inner.selection); - let artist_id = &artist.meta.id.clone(); + // Find the artist in the full collection to correctly identify already existing albums. + let artist_id = artist.meta.id.clone(); + let coll = inner.music_hoard.get_collection(); + let artist = coll.iter().find(|a| a.meta.id == artist_id).unwrap(); + for new in Self::new_albums(fetch_albums, &artist.albums).into_iter() { - inner.music_hoard.add_album(artist_id, new)?; + inner.music_hoard.add_album(&artist_id, new)?; } - let coll = inner.music_hoard.get_collection(); + let coll = inner.music_hoard.get_filtered(); inner.selection.select_by_id(coll, selection); Ok(()) diff --git a/src/tui/app/machine/mod.rs b/src/tui/app/machine/mod.rs index 2d0322f..6f630a5 100644 --- a/src/tui/app/machine/mod.rs +++ b/src/tui/app/machine/mod.rs @@ -173,7 +173,7 @@ impl AppInner { music_hoard: MH, musicbrainz: MB, ) -> Self { - let selection = Selection::new(music_hoard.get_collection()); + let selection = Selection::new(music_hoard.get_filtered()); AppInner { running: true, music_hoard: Box::new(music_hoard), @@ -186,7 +186,7 @@ impl AppInner { impl<'a> From<&'a mut AppInner> for AppPublicInner<'a> { fn from(inner: &'a mut AppInner) -> Self { AppPublicInner { - collection: inner.music_hoard.get_collection(), + collection: inner.music_hoard.get_filtered(), selection: &mut inner.selection, } } @@ -330,7 +330,7 @@ mod tests { pub fn music_hoard(collection: Collection) -> MockIMusicHoard { let mut music_hoard = MockIMusicHoard::new(); - music_hoard.expect_get_collection().return_const(collection); + music_hoard.expect_get_filtered().return_const(collection); music_hoard } @@ -594,7 +594,7 @@ mod tests { .expect_rescan_library() .times(1) .return_once(|| Err(musichoard::Error::LibraryError(String::from("get rekt")))); - music_hoard.expect_get_collection().return_const(vec![]); + music_hoard.expect_get_filtered().return_const(vec![]); let app = App::new(music_hoard, mb_job_sender()); assert!(app.is_running()); diff --git a/src/tui/app/machine/reload_state.rs b/src/tui/app/machine/reload_state.rs index 25def5b..deea728 100644 --- a/src/tui/app/machine/reload_state.rs +++ b/src/tui/app/machine/reload_state.rs @@ -29,7 +29,7 @@ impl IAppInteractReload for AppMachine { fn reload_library(mut self) -> Self::APP { let previous = KeySelection::get( - self.inner.music_hoard.get_collection(), + self.inner.music_hoard.get_filtered(), &self.inner.selection, ); let result = self.inner.music_hoard.rescan_library(); @@ -38,7 +38,7 @@ impl IAppInteractReload for AppMachine { fn reload_database(mut self) -> Self::APP { let previous = KeySelection::get( - self.inner.music_hoard.get_collection(), + self.inner.music_hoard.get_filtered(), &self.inner.selection, ); let result = self.inner.music_hoard.reload_database(); @@ -60,7 +60,7 @@ impl IAppInteractReloadPrivate for AppMachine { Ok(()) => { self.inner .selection - .select_by_id(self.inner.music_hoard.get_collection(), previous); + .select_by_id(self.inner.music_hoard.get_filtered(), previous); AppMachine::browse_state(self.inner).into() } Err(err) => AppMachine::error_state(self.inner, err.to_string()).into(), diff --git a/src/tui/app/machine/search_state.rs b/src/tui/app/machine/search_state.rs index 736bcec..1e4dc0d 100644 --- a/src/tui/app/machine/search_state.rs +++ b/src/tui/app/machine/search_state.rs @@ -66,7 +66,7 @@ impl IAppInteractSearch for AppMachine { } fn step_back(mut self) -> Self::APP { - let collection = self.inner.music_hoard.get_collection(); + let collection = self.inner.music_hoard.get_filtered(); if let Some(memo) = self.state.memo.pop() { if memo.char { self.state.string.pop(); @@ -104,7 +104,7 @@ trait IAppInteractSearchPrivate { impl IAppInteractSearchPrivate for AppMachine { fn incremental_search(&mut self, next: bool) { - let collection = self.inner.music_hoard.get_collection(); + let collection = self.inner.music_hoard.get_filtered(); let search = &self.state.string; let sel = &self.inner.selection; @@ -121,7 +121,7 @@ impl IAppInteractSearchPrivate for AppMachine { }; if result.is_some() { - let collection = self.inner.music_hoard.get_collection(); + let collection = self.inner.music_hoard.get_filtered(); self.inner.selection.select(collection, result); } } diff --git a/src/tui/lib/mod.rs b/src/tui/lib/mod.rs index c08eb3a..ef67eec 100644 --- a/src/tui/lib/mod.rs +++ b/src/tui/lib/mod.rs @@ -18,6 +18,8 @@ use mockall::automock; pub trait IMusicHoard { fn rescan_library(&mut self) -> Result<(), musichoard::Error>; fn reload_database(&mut self) -> Result<(), musichoard::Error>; + + fn get_filtered(&self) -> &Collection; fn get_collection(&self) -> &Collection; fn add_album( @@ -65,6 +67,10 @@ impl IMusicHoard for MusicHoard::reload_database(self) } + fn get_filtered(&self) -> &Collection { + ::get_filtered(self) + } + fn get_collection(&self) -> &Collection { ::get_collection(self) } diff --git a/src/tui/mod.rs b/src/tui/mod.rs index 2a2fd80..630fcb5 100644 --- a/src/tui/mod.rs +++ b/src/tui/mod.rs @@ -200,7 +200,7 @@ mod tests { music_hoard.expect_reload_database().returning(|| Ok(())); music_hoard.expect_rescan_library().returning(|| Ok(())); - music_hoard.expect_get_collection().return_const(collection); + music_hoard.expect_get_filtered().return_const(collection); music_hoard } -- 2.47.1 From b7779905bcf0e2ee81297b47bb944f14af33c5c1 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 20:45:05 +0100 Subject: [PATCH 07/14] Fix unit tests --- src/tui/app/machine/fetch_state.rs | 20 +++++++++++++++++--- src/tui/app/machine/match_state.rs | 24 +++++++++++++++++++++--- 2 files changed, 38 insertions(+), 6 deletions(-) diff --git a/src/tui/app/machine/fetch_state.rs b/src/tui/app/machine/fetch_state.rs index 412c9f1..55eb380 100644 --- a/src/tui/app/machine/fetch_state.rs +++ b/src/tui/app/machine/fetch_state.rs @@ -365,7 +365,7 @@ impl IAppEventFetch for AppMachine { #[cfg(test)] mod tests { - use mockall::predicate; + use mockall::{predicate, Sequence}; use musichoard::collection::{ album::AlbumMeta, artist::{ArtistId, ArtistMeta}, @@ -739,11 +739,18 @@ mod tests { tx.send(fetch_result).unwrap(); drop(tx); - let mut music_hoard = music_hoard(collection); + let mut music_hoard = music_hoard(collection.clone()); + let mut seq = Sequence::new(); + music_hoard + .expect_get_collection() + .times(1) + .in_sequence(&mut seq) + .return_const(collection); music_hoard .expect_add_album() .with(predicate::eq(artist_id), predicate::eq(new_album)) .times(1) + .in_sequence(&mut seq) .return_once(|_, _| Ok(())); let app = AppMachine::app_fetch_next(inner(music_hoard), fetch); @@ -766,11 +773,18 @@ mod tests { tx.send(fetch_result).unwrap(); drop(tx); - let mut music_hoard = music_hoard(collection); + let mut music_hoard = music_hoard(collection.clone()); + let mut seq = Sequence::new(); + music_hoard + .expect_get_collection() + .times(1) + .in_sequence(&mut seq) + .return_const(collection); music_hoard .expect_add_album() .with(predicate::eq(artist_id), predicate::eq(new_album)) .times(1) + .in_sequence(&mut seq) .return_once(|_, _| Err(musichoard::Error::CollectionError(String::from("get rekt")))); let app = AppMachine::app_fetch_next(inner(music_hoard), fetch); diff --git a/src/tui/app/machine/match_state.rs b/src/tui/app/machine/match_state.rs index 17eec10..4c8e83c 100644 --- a/src/tui/app/machine/match_state.rs +++ b/src/tui/app/machine/match_state.rs @@ -447,7 +447,8 @@ mod tests { let (_tx, rx) = mpsc::channel(); let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); - let mut music_hoard = music_hoard(vec![]); + let collection = vec![]; + let mut music_hoard = music_hoard(collection.clone()); let artist_id = ArtistId::new("Artist"); match matches_info { EntityMatches::Album(_) => { @@ -456,6 +457,11 @@ mod tests { let info = AlbumInfo::default(); let mut seq = Sequence::new(); + music_hoard + .expect_get_collection() + .times(1) + .in_sequence(&mut seq) + .return_const(collection); music_hoard .expect_merge_album_info() .with(eq(artist_id.clone()), eq(album_id.clone()), eq(info)) @@ -595,7 +601,8 @@ mod tests { let (_tx, rx) = mpsc::channel(); let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); - let mut music_hoard = music_hoard(vec![]); + let collection = vec![]; + let mut music_hoard = music_hoard(collection.clone()); match matches_info { EntityMatches::Artist(_) => panic!(), EntityMatches::Album(matches) => { @@ -606,6 +613,11 @@ mod tests { let artist = matches.artist.clone(); let mut seq = Sequence::new(); + music_hoard + .expect_get_collection() + .times(1) + .in_sequence(&mut seq) + .return_const(collection); music_hoard .expect_merge_album_info() .with(eq(artist.clone()), eq(album_id.clone()), eq(meta.info)) @@ -647,13 +659,19 @@ mod tests { let (_tx, rx) = mpsc::channel(); let app_matches = MatchState::new(matches_info.clone(), FetchState::search(rx)); - let mut music_hoard = music_hoard(vec![artist]); + let collection = vec![artist]; + let mut music_hoard = music_hoard(collection.clone()); match matches_info { EntityMatches::Artist(_) => panic!(), EntityMatches::Album(matches) => { let artist = matches.artist.clone(); let mut seq = Sequence::new(); + music_hoard + .expect_get_collection() + .times(1) + .in_sequence(&mut seq) + .return_const(collection); music_hoard .expect_remove_album() .times(1) -- 2.47.1 From f3f35826019ab4450c70471d2635df00254e07f0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 21:34:06 +0100 Subject: [PATCH 08/14] Fix filtering of unowned albums --- src/core/musichoard/filter.rs | 58 ++++++++++++++++++++++++++--------- src/main.rs | 41 +++++++++++++++---------- 2 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs index 203a95a..5452d58 100644 --- a/src/core/musichoard/filter.rs +++ b/src/core/musichoard/filter.rs @@ -3,8 +3,8 @@ use crate::core::collection::album::{Album, AlbumOwnership, AlbumPrimaryType, Al /// Filter for a specifying subsets of the entire collection (e.g., for display). #[derive(Debug, Default)] pub struct CollectionFilter { - pub include: Vec, - pub except: Vec, + pub include: Vec>, + pub except: Vec>, } #[derive(Debug)] @@ -16,11 +16,19 @@ pub enum AlbumField { impl CollectionFilter { pub fn filter_album(&self, album: &Album) -> bool { - let include = Self::filter_or(&self.include, album); - let except = Self::filter_or(&self.except, album); + let include = Self::filter_and(true, &self.include, album); + let except = Self::filter_and(false, &self.except, album); include && !except } + fn filter_and(empty: bool, group: &Vec>, album: &Album) -> bool { + let mut filter = !group.is_empty() || empty; + for field in group.iter() { + filter = filter && Self::filter_or(field, album); + } + filter + } + fn filter_or(group: &Vec, album: &Album) -> bool { let mut filter = false; for field in group.iter() { @@ -52,17 +60,21 @@ mod tests { fn test_filter() -> CollectionFilter { CollectionFilter { - include: vec![ + include: vec![vec![ AlbumField::PrimaryType(None), AlbumField::PrimaryType(Some(AlbumPrimaryType::Ep)), AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), - ], + AlbumField::Ownership(AlbumOwnership::Owned(TrackFormat::Mp3)), + AlbumField::Ownership(AlbumOwnership::Owned(TrackFormat::Flac)), + ]], except: vec![ - AlbumField::Ownership(AlbumOwnership::None), - AlbumField::SecondaryType(AlbumSecondaryType::Compilation), - AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), - AlbumField::SecondaryType(AlbumSecondaryType::Live), - AlbumField::SecondaryType(AlbumSecondaryType::Demo), + vec![AlbumField::Ownership(AlbumOwnership::None)], + vec![ + AlbumField::SecondaryType(AlbumSecondaryType::Compilation), + AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), + AlbumField::SecondaryType(AlbumSecondaryType::Live), + AlbumField::SecondaryType(AlbumSecondaryType::Demo), + ], ], } } @@ -93,6 +105,9 @@ mod tests { let filter = test_filter(); let mut album = test_album(); + // Drop ownership so that filtering is truly only on type. + album.tracks.clear(); + album.meta.info.primary_type = None; assert!(filter.filter_album(&album)); @@ -121,29 +136,42 @@ mod tests { types.push(AlbumSecondaryType::AudioDrama); assert!(filter.filter_album(&album)); - // Filtered type. + // Filtered type. But album is owned so it remains. let types = &mut album.meta.info.secondary_types; types.push(AlbumSecondaryType::Live); - assert!(!filter.filter_album(&album)); + assert_ne!(album.get_ownership(), AlbumOwnership::None); + assert!(filter.filter_album(&album)); // Remove non-filtered type. album.meta.info.secondary_types.remove(0); - assert!(!filter.filter_album(&album)); + assert!(filter.filter_album(&album)); // Add another filtered type. let types = &mut album.meta.info.secondary_types; types.push(AlbumSecondaryType::Soundtrack); + assert!(filter.filter_album(&album)); + + // Drop ownership and this should be now excluded. + album.tracks.clear(); + assert_eq!(album.get_ownership(), AlbumOwnership::None); assert!(!filter.filter_album(&album)); } #[test] fn filter_ownership() { let filter = test_filter(); - let mut album = Album::new(AlbumId::new("An Album")); + let mut album = test_album(); + // It is an album though so it should remain included. album.tracks.clear(); + assert_eq!(album.get_ownership(), AlbumOwnership::None); + assert!(filter.filter_album(&album)); + + // Change to unincluded primary type. + album.meta.info.primary_type = Some(AlbumPrimaryType::Other); assert!(!filter.filter_album(&album)); + // Changing ownership should make it go back to being included. album.tracks.push(test_track()); assert_eq!( album.get_ownership(), diff --git a/src/main.rs b/src/main.rs index 760a268..3b76b08 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,10 @@ use ratatui::{backend::CrosstermBackend, Terminal}; use structopt::StructOpt; use musichoard::{ - collection::album::{AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}, + collection::{ + album::{AlbumOwnership, AlbumPrimaryType, AlbumSecondaryType}, + track::TrackFormat, + }, external::{ database::json::{backend::JsonDatabaseFileBackend, JsonDatabase}, library::beets::{ @@ -73,25 +76,29 @@ struct DbOpt { fn default_filter() -> CollectionFilter { CollectionFilter { - include: vec![ + include: vec![vec![ AlbumField::PrimaryType(None), AlbumField::PrimaryType(Some(AlbumPrimaryType::Ep)), AlbumField::PrimaryType(Some(AlbumPrimaryType::Album)), - ], + AlbumField::Ownership(AlbumOwnership::Owned(TrackFormat::Mp3)), + AlbumField::Ownership(AlbumOwnership::Owned(TrackFormat::Flac)), + ]], except: vec![ - AlbumField::SecondaryType(AlbumSecondaryType::Compilation), - AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), - AlbumField::SecondaryType(AlbumSecondaryType::Spokenword), - AlbumField::SecondaryType(AlbumSecondaryType::Interview), - AlbumField::SecondaryType(AlbumSecondaryType::Audiobook), - AlbumField::SecondaryType(AlbumSecondaryType::AudioDrama), - AlbumField::SecondaryType(AlbumSecondaryType::Live), - AlbumField::SecondaryType(AlbumSecondaryType::Remix), - AlbumField::SecondaryType(AlbumSecondaryType::DjMix), - AlbumField::SecondaryType(AlbumSecondaryType::MixtapeStreet), - AlbumField::SecondaryType(AlbumSecondaryType::Demo), - AlbumField::SecondaryType(AlbumSecondaryType::FieldRecording), - AlbumField::Ownership(AlbumOwnership::None), + vec![AlbumField::Ownership(AlbumOwnership::None)], + vec![ + AlbumField::SecondaryType(AlbumSecondaryType::Compilation), + AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), + AlbumField::SecondaryType(AlbumSecondaryType::Spokenword), + AlbumField::SecondaryType(AlbumSecondaryType::Interview), + AlbumField::SecondaryType(AlbumSecondaryType::Audiobook), + AlbumField::SecondaryType(AlbumSecondaryType::AudioDrama), + AlbumField::SecondaryType(AlbumSecondaryType::Live), + AlbumField::SecondaryType(AlbumSecondaryType::Remix), + AlbumField::SecondaryType(AlbumSecondaryType::DjMix), + AlbumField::SecondaryType(AlbumSecondaryType::MixtapeStreet), + AlbumField::SecondaryType(AlbumSecondaryType::Demo), + AlbumField::SecondaryType(AlbumSecondaryType::FieldRecording), + ], ], } } @@ -100,7 +107,7 @@ fn with( builder: MusicHoardBuilder, ) { let mut music_hoard = builder.build().expect("failed to initialise MusicHoard"); - music_hoard.set_filter(default_filter()); + // music_hoard.set_filter(default_filter()); // Initialize the terminal user interface. let backend = CrosstermBackend::new(io::stdout()); -- 2.47.1 From 8a42f7c71299eb06fa3238607217af697ec82ede Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 21:51:26 +0100 Subject: [PATCH 09/14] Fix --- src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index 3b76b08..502a2be 100644 --- a/src/main.rs +++ b/src/main.rs @@ -107,7 +107,7 @@ fn with( builder: MusicHoardBuilder, ) { let mut music_hoard = builder.build().expect("failed to initialise MusicHoard"); - // music_hoard.set_filter(default_filter()); + music_hoard.set_filter(default_filter()); // Initialize the terminal user interface. let backend = CrosstermBackend::new(io::stdout()); -- 2.47.1 From e9b169fdef83dbe0d43e2d096862d1380ce55dd5 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 22:13:43 +0100 Subject: [PATCH 10/14] Add convenience method --- src/core/collection/album.rs | 19 +++++++++++++++++++ src/main.rs | 19 +++++-------------- 2 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index e5d0033..bb128e0 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -137,6 +137,25 @@ pub enum AlbumSecondaryType { FieldRecording, } +impl AlbumSecondaryType { + pub fn all_variants() -> [AlbumSecondaryType; 12] { + [ + AlbumSecondaryType::Compilation, + AlbumSecondaryType::Soundtrack, + AlbumSecondaryType::Spokenword, + AlbumSecondaryType::Interview, + AlbumSecondaryType::Audiobook, + AlbumSecondaryType::AudioDrama, + AlbumSecondaryType::Live, + AlbumSecondaryType::Remix, + AlbumSecondaryType::DjMix, + AlbumSecondaryType::MixtapeStreet, + AlbumSecondaryType::Demo, + AlbumSecondaryType::FieldRecording, + ] + } +} + /// The album's ownership status. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum AlbumOwnership { diff --git a/src/main.rs b/src/main.rs index 502a2be..c3cb2be 100644 --- a/src/main.rs +++ b/src/main.rs @@ -85,20 +85,11 @@ fn default_filter() -> CollectionFilter { ]], except: vec![ vec![AlbumField::Ownership(AlbumOwnership::None)], - vec![ - AlbumField::SecondaryType(AlbumSecondaryType::Compilation), - AlbumField::SecondaryType(AlbumSecondaryType::Soundtrack), - AlbumField::SecondaryType(AlbumSecondaryType::Spokenword), - AlbumField::SecondaryType(AlbumSecondaryType::Interview), - AlbumField::SecondaryType(AlbumSecondaryType::Audiobook), - AlbumField::SecondaryType(AlbumSecondaryType::AudioDrama), - AlbumField::SecondaryType(AlbumSecondaryType::Live), - AlbumField::SecondaryType(AlbumSecondaryType::Remix), - AlbumField::SecondaryType(AlbumSecondaryType::DjMix), - AlbumField::SecondaryType(AlbumSecondaryType::MixtapeStreet), - AlbumField::SecondaryType(AlbumSecondaryType::Demo), - AlbumField::SecondaryType(AlbumSecondaryType::FieldRecording), - ], + AlbumSecondaryType::all_variants() + .iter() + .cloned() + .map(AlbumField::SecondaryType) + .collect(), ], } } -- 2.47.1 From de5888e9a0f7c0380b061128662c7535a21fc223 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 22:15:16 +0100 Subject: [PATCH 11/14] fix --- src/core/collection/album.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/core/collection/album.rs b/src/core/collection/album.rs index bb128e0..9e5c7e4 100644 --- a/src/core/collection/album.rs +++ b/src/core/collection/album.rs @@ -473,4 +473,12 @@ mod tests { let expected = meta.clone().with_date(right.date); assert_eq!(expected, left.merge(right)); } + + #[test] + fn secondary_types_all_variants() { + let mut variants = AlbumSecondaryType::all_variants().to_vec(); + variants.sort_unstable(); + variants.dedup(); + assert_eq!(variants, AlbumSecondaryType::all_variants().to_vec()); + } } -- 2.47.1 From e136103eade0b55d9da22d3220968ab213f3ea00 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 22:23:34 +0100 Subject: [PATCH 12/14] Complete coverage --- src/core/musichoard/base.rs | 34 +++++++++++++++++++++++++++++++++- src/core/musichoard/filter.rs | 2 +- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index db8f910..1ec0534 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -148,7 +148,14 @@ impl IMusicHoardBasePrivate for MusicHoard #[cfg(test)] mod tests { - use crate::core::testmod::FULL_COLLECTION; + use crate::{ + collection::{ + album::{AlbumMeta, AlbumPrimaryType}, + artist::ArtistMeta, + }, + core::testmod::FULL_COLLECTION, + filter::AlbumField, + }; use super::*; @@ -321,4 +328,29 @@ mod tests { mh.collection = mh.merge_collections(); assert_eq!(expected, mh.collection); } + + #[test] + fn filtered() { + let mut mh = MusicHoard::default(); + mh.collection = vec![Artist { + meta: ArtistMeta::new(ArtistId::new("Artist")), + albums: vec![ + Album::new(AlbumId::new("Album 1")), + Album::new(AlbumId::new("Album 2")), + ], + }]; + mh.collection[0].albums[0].meta.info.primary_type = Some(AlbumPrimaryType::Ep); + mh.collection[0].albums[0].meta.info.primary_type = Some(AlbumPrimaryType::Album); + + mh.set_filter(CollectionFilter { + include: vec![vec![AlbumField::PrimaryType(Some(AlbumPrimaryType::Album))]], + except: vec![], + }); + + assert_eq!(mh.get_collection().len(), 1); + assert_eq!(mh.get_filtered().len(), 1); + + assert_eq!(mh.get_collection()[0].albums.len(), 2); + assert_eq!(mh.get_filtered()[0].albums.len(), 1); + } } diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs index 5452d58..8e9f21e 100644 --- a/src/core/musichoard/filter.rs +++ b/src/core/musichoard/filter.rs @@ -58,7 +58,7 @@ mod tests { use super::*; - fn test_filter() -> CollectionFilter { + pub fn test_filter() -> CollectionFilter { CollectionFilter { include: vec![vec![ AlbumField::PrimaryType(None), -- 2.47.1 From 19f201e1ac541bfd022a57437ad8a41f8acf4962 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 22:31:02 +0100 Subject: [PATCH 13/14] Lints --- src/core/musichoard/base.rs | 25 ++++++++++++------------- src/core/musichoard/builder.rs | 4 +++- src/core/musichoard/filter.rs | 6 +++--- src/tui/app/machine/reload_state.rs | 12 ++++-------- 4 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/core/musichoard/base.rs b/src/core/musichoard/base.rs index 1ec0534..08a04c7 100644 --- a/src/core/musichoard/base.rs +++ b/src/core/musichoard/base.rs @@ -89,7 +89,7 @@ impl IMusicHoardBasePrivate for MusicHoard fn filter_collection(&self) -> Collection { let iter = self.collection.iter(); - iter.map(|a| self.filter_artist(a)).flatten().collect() + iter.flat_map(|a| self.filter_artist(a)).collect() } fn filter_artist(&self, artist: &Artist) -> Option { @@ -149,10 +149,7 @@ impl IMusicHoardBasePrivate for MusicHoard #[cfg(test)] mod tests { use crate::{ - collection::{ - album::{AlbumMeta, AlbumPrimaryType}, - artist::ArtistMeta, - }, + collection::{album::AlbumPrimaryType, artist::ArtistMeta}, core::testmod::FULL_COLLECTION, filter::AlbumField, }; @@ -331,14 +328,16 @@ mod tests { #[test] fn filtered() { - let mut mh = MusicHoard::default(); - mh.collection = vec![Artist { - meta: ArtistMeta::new(ArtistId::new("Artist")), - albums: vec![ - Album::new(AlbumId::new("Album 1")), - Album::new(AlbumId::new("Album 2")), - ], - }]; + let mut mh = MusicHoard { + collection: vec![Artist { + meta: ArtistMeta::new(ArtistId::new("Artist")), + albums: vec![ + Album::new(AlbumId::new("Album 1")), + Album::new(AlbumId::new("Album 2")), + ], + }], + ..Default::default() + }; mh.collection[0].albums[0].meta.info.primary_type = Some(AlbumPrimaryType::Ep); mh.collection[0].albums[0].meta.info.primary_type = Some(AlbumPrimaryType::Album); diff --git a/src/core/musichoard/builder.rs b/src/core/musichoard/builder.rs index a68aac9..1901a9f 100644 --- a/src/core/musichoard/builder.rs +++ b/src/core/musichoard/builder.rs @@ -1,6 +1,8 @@ use crate::core::{ interface::{database::IDatabase, library::ILibrary}, - musichoard::{database::IMusicHoardDatabase, Error, MusicHoard, NoDatabase, NoLibrary, CollectionFilter}, + musichoard::{ + database::IMusicHoardDatabase, CollectionFilter, Error, MusicHoard, NoDatabase, NoLibrary, + }, }; /// Builder for [`MusicHoard`]. Its purpose is to make it easier to set various combinations of diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs index 8e9f21e..c1b317f 100644 --- a/src/core/musichoard/filter.rs +++ b/src/core/musichoard/filter.rs @@ -21,7 +21,7 @@ impl CollectionFilter { include && !except } - fn filter_and(empty: bool, group: &Vec>, album: &Album) -> bool { + fn filter_and(empty: bool, group: &[Vec], album: &Album) -> bool { let mut filter = !group.is_empty() || empty; for field in group.iter() { filter = filter && Self::filter_or(field, album); @@ -29,7 +29,7 @@ impl CollectionFilter { filter } - fn filter_or(group: &Vec, album: &Album) -> bool { + fn filter_or(group: &[AlbumField], album: &Album) -> bool { let mut filter = false; for field in group.iter() { filter = filter || Self::filter_field(field, album); @@ -42,7 +42,7 @@ impl CollectionFilter { AlbumField::PrimaryType(filter) => *filter == album.meta.info.primary_type, AlbumField::SecondaryType(filter) => { let types = &album.meta.info.secondary_types; - types.iter().find(|st| *st == filter).is_some() + types.iter().any(|st| st == filter) } AlbumField::Ownership(filter) => *filter == album.get_ownership(), } diff --git a/src/tui/app/machine/reload_state.rs b/src/tui/app/machine/reload_state.rs index deea728..7be5033 100644 --- a/src/tui/app/machine/reload_state.rs +++ b/src/tui/app/machine/reload_state.rs @@ -28,19 +28,15 @@ impl IAppInteractReload for AppMachine { type APP = App; fn reload_library(mut self) -> Self::APP { - let previous = KeySelection::get( - self.inner.music_hoard.get_filtered(), - &self.inner.selection, - ); + let previous = + KeySelection::get(self.inner.music_hoard.get_filtered(), &self.inner.selection); let result = self.inner.music_hoard.rescan_library(); self.refresh(previous, result) } fn reload_database(mut self) -> Self::APP { - let previous = KeySelection::get( - self.inner.music_hoard.get_filtered(), - &self.inner.selection, - ); + let previous = + KeySelection::get(self.inner.music_hoard.get_filtered(), &self.inner.selection); let result = self.inner.music_hoard.reload_database(); self.refresh(previous, result) } -- 2.47.1 From 91b671c66825ca57ae51d5fab8a9bd4875d8c983 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 4 Jan 2025 22:38:19 +0100 Subject: [PATCH 14/14] Update UT --- src/core/musichoard/filter.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/core/musichoard/filter.rs b/src/core/musichoard/filter.rs index c1b317f..9e56011 100644 --- a/src/core/musichoard/filter.rs +++ b/src/core/musichoard/filter.rs @@ -107,6 +107,7 @@ mod tests { // Drop ownership so that filtering is truly only on type. album.tracks.clear(); + assert_eq!(album.get_ownership(), AlbumOwnership::None); album.meta.info.primary_type = None; assert!(filter.filter_album(&album)); @@ -142,10 +143,6 @@ mod tests { assert_ne!(album.get_ownership(), AlbumOwnership::None); assert!(filter.filter_album(&album)); - // Remove non-filtered type. - album.meta.info.secondary_types.remove(0); - assert!(filter.filter_album(&album)); - // Add another filtered type. let types = &mut album.meta.info.secondary_types; types.push(AlbumSecondaryType::Soundtrack); @@ -155,6 +152,14 @@ mod tests { album.tracks.clear(); assert_eq!(album.get_ownership(), AlbumOwnership::None); assert!(!filter.filter_album(&album)); + + // Remove one of the two filtered types. + album.meta.info.secondary_types.pop(); + assert!(!filter.filter_album(&album)); + + // Remove the second filtered type. Should now be included by primary type. + album.meta.info.secondary_types.pop(); + assert!(filter.filter_album(&album)); } #[test] -- 2.47.1