Replace Query and QueryOption with better API (#35)

Closes #34

Reviewed-on: https://git.wojciechkozlowski.eu/wojtek/musichoard/pulls/35

src/collection/mod.rs

@@ -75,7 +75,7 @@ impl<LIB: Library, DB: Database> MhCollectionManager<LIB, DB> {
 
 impl<LIB: Library, DB: Database> CollectionManager for MhCollectionManager<LIB, DB> {
     fn rescan_library(&mut self) -> Result<(), Error> {
-        self.collection = self.library.list(&Query::default())?;
+        self.collection = self.library.list(&Query::new())?;
         Ok(())
     }
 
@@ -107,7 +107,7 @@ mod tests {
         let mut library = MockLibrary::new();
         let mut database = MockDatabase::new();
 
-        let library_input = Query::default();
+        let library_input = Query::new();
         let library_result = Ok(COLLECTION.to_owned());
 
         let database_input = COLLECTION.to_owned();

src/library/beets.rs

@@ -3,7 +3,6 @@
 
 use std::{
     collections::{HashMap, HashSet},
-    fmt::Display,
     path::{Path, PathBuf},
     process::Command,
     str,
@@ -14,7 +13,7 @@ use mockall::automock;
 
 use crate::{Album, AlbumId, Artist, ArtistId, Track, TrackFormat};
 
-use super::{Error, Library, Query, QueryOption};
+use super::{Error, Field, Library, Query};
 
 macro_rules! list_format_separator {
     () => {
@@ -43,71 +42,35 @@ const LIST_FORMAT_ARG: &str = concat!(
 const TRACK_FORMAT_FLAC: &str = "FLAC";
 const TRACK_FORMAT_MP3: &str = "MP3";
 
-trait QueryOptionArgBeets {
+trait ToBeetsArg {
-    fn to_arg(&self, option_name: &str) -> Option<String>;
+    fn to_arg(&self, include: bool) -> String;
 }
 
-trait QueryArgsBeets {
+trait ToBeetsArgs {
     fn to_args(&self) -> Vec<String>;
 }
 
-trait SimpleOption {}
+impl ToBeetsArg for Field {
-impl SimpleOption for String {}
+    fn to_arg(&self, include: bool) -> String {
-impl SimpleOption for u32 {}
+        let negate = if include { "" } else { "^" };
-
+        match self {
-impl<T: SimpleOption + Display> QueryOptionArgBeets for QueryOption<T> {
+            Field::AlbumArtist(ref s) => format!("{negate}albumartist:{s}"),
-    fn to_arg(&self, option_name: &str) -> Option<String> {
+            Field::AlbumYear(ref u) => format!("{negate}year:{u}"),
-        let (negate, value) = match self {
+            Field::AlbumTitle(ref s) => format!("{negate}album:{s}"),
-            Self::Include(value) => ("", value),
+            Field::TrackNumber(ref u) => format!("{negate}track:{u}"),
-            Self::Exclude(value) => ("^", value),
+            Field::TrackTitle(ref s) => format!("{negate}title:{s}"),
-            Self::None => return None,
+            Field::TrackArtist(ref v) => format!("{negate}artist:{}", v.join("; ")),
-        };
+            Field::All(ref s) => format!("{negate}{s}"),
-        Some(format!("{negate}{option_name}{value}"))
+        }
     }
 }
 
-impl QueryOptionArgBeets for QueryOption<Vec<String>> {
+impl ToBeetsArgs for Query {
-    fn to_arg(&self, option_name: &str) -> Option<String> {
-        let (negate, vec) = match self {
-            Self::Include(value) => ("", value),
-            Self::Exclude(value) => ("^", value),
-            Self::None => return None,
-        };
-        Some(format!("{negate}{option_name}{}", vec.join("; ")))
-    }
-}
-
-impl QueryArgsBeets for Query {
     fn to_args(&self) -> Vec<String> {
         let mut arguments: Vec<String> = vec![];
 
-        if let Some(album_artist) = self.album_artist.to_arg("albumartist:") {
+        arguments.extend(self.include.iter().map(|field| field.to_arg(true)));
-            arguments.push(album_artist);
+        arguments.extend(self.exclude.iter().map(|field| field.to_arg(false)));
-        };
-
-        if let Some(album_year) = self.album_year.to_arg("year:") {
-            arguments.push(album_year);
-        };
-
-        if let Some(album_title) = self.album_title.to_arg("album:") {
-            arguments.push(album_title);
-        };
-
-        if let Some(track_number) = self.track_number.to_arg("track:") {
-            arguments.push(track_number);
-        };
-
-        if let Some(track_title) = self.track_title.to_arg("title:") {
-            arguments.push(track_title);
-        };
-
-        if let Some(track_artist) = self.track_artist.to_arg("artist:") {
-            arguments.push(track_artist);
-        };
-
-        if let Some(all) = self.all.to_arg("") {
-            arguments.push(all);
-        }
 
         arguments
     }
@@ -328,41 +291,45 @@ mod tests {
 
     #[test]
     fn test_query() {
-        let query = Query::new()
+        let mut query = Query::new()
-            .album_title(QueryOption::Exclude(String::from("some.album")))
+            .exclude(Field::AlbumTitle(String::from("some.album")))
-            .track_number(QueryOption::Include(5))
+            .include(Field::TrackNumber(5))
-            .track_artist(QueryOption::Include(vec![
+            .include(Field::TrackArtist(vec![
                 String::from("some.artist.1"),
                 String::from("some.artist.2"),
             ]))
-            .all(QueryOption::Exclude(String::from("some.all")));
+            .exclude(Field::All(String::from("some.all")))
+            .to_args();
+        query.sort();
 
         assert_eq!(
-            query.to_args(),
+            query,
             vec![
                 String::from("^album:some.album"),
-                String::from("track:5"),
-                String::from("artist:some.artist.1; some.artist.2"),
                 String::from("^some.all"),
+                String::from("artist:some.artist.1; some.artist.2"),
+                String::from("track:5"),
             ]
         );
 
-        let query = Query::new()
+        let mut query = Query::default()
-            .album_artist(QueryOption::Exclude(String::from("some.albumartist")))
+            .exclude(Field::AlbumArtist(String::from("some.albumartist")))
-            .album_year(QueryOption::Include(3030))
+            .include(Field::AlbumYear(3030))
-            .track_title(QueryOption::Include(String::from("some.track")))
+            .include(Field::TrackTitle(String::from("some.track")))
-            .track_artist(QueryOption::Exclude(vec![
+            .exclude(Field::TrackArtist(vec![
                 String::from("some.artist.1"),
                 String::from("some.artist.2"),
-            ]));
+            ]))
+            .to_args();
+        query.sort();
 
         assert_eq!(
-            query.to_args(),
+            query,
             vec![
                 String::from("^albumartist:some.albumartist"),
-                String::from("year:3030"),
-                String::from("title:some.track"),
                 String::from("^artist:some.artist.1; some.artist.2"),
+                String::from("title:some.track"),
+                String::from("year:3030"),
             ]
         );
     }
@@ -380,7 +347,7 @@ mod tests {
             .return_once(|_| result);
 
         let mut beets = BeetsLibrary::new(executor);
-        let output = beets.list(&Query::default()).unwrap();
+        let output = beets.list(&Query::new()).unwrap();
 
         let expected: Vec<Artist> = vec![];
         assert_eq!(output, expected);
@@ -400,7 +367,7 @@ mod tests {
             .return_once(|_| result);
 
         let mut beets = BeetsLibrary::new(executor);
-        let output = beets.list(&Query::default()).unwrap();
+        let output = beets.list(&Query::new()).unwrap();
 
         assert_eq!(output, expected);
     }
@@ -435,7 +402,7 @@ mod tests {
             .return_once(|_| result);
 
         let mut beets = BeetsLibrary::new(executor);
-        let output = beets.list(&Query::default()).unwrap();
+        let output = beets.list(&Query::new()).unwrap();
 
         assert_eq!(output, expected);
     }
@@ -457,31 +424,36 @@ mod tests {
             .return_once(|_| result);
 
         let mut beets = BeetsLibrary::new(executor);
-        let output = beets.list(&Query::default()).unwrap();
+        let output = beets.list(&Query::new()).unwrap();
 
         assert_eq!(output, expected);
     }
 
     #[test]
     fn test_list_query() {
-        let query = Query::new()
+        let mut query = Query::new();
-            .album_title(QueryOption::Exclude(String::from("some.album")))
+        query
-            .track_number(QueryOption::Include(5))
+            .exclude(Field::AlbumTitle(String::from("some.album")))
-            .track_artist(QueryOption::Include(vec![String::from("some.artist")]));
+            .include(Field::TrackNumber(5))
+            .include(Field::TrackArtist(vec![String::from("some.artist")]));
 
         let arguments = vec![
             "ls".to_string(),
             LIST_FORMAT_ARG.to_string(),
             String::from("^album:some.album"),
-            String::from("track:5"),
             String::from("artist:some.artist"),
+            String::from("track:5"),
         ];
         let result = Ok(vec![]);
 
         let mut executor = MockBeetsLibraryExecutor::new();
         executor
             .expect_exec()
-            .with(predicate::eq(arguments))
+            .with(predicate::function(move |x: &[String]| {
+                let mut y = x.to_owned();
+                y[2..].sort();
+                y == arguments
+            }))
             .times(1)
             .return_once(|_| result);
 
@@ -513,7 +485,7 @@ mod tests {
             .return_once(|_| result);
 
         let mut beets = BeetsLibrary::new(executor);
-        let err = beets.list(&Query::default()).unwrap_err();
+        let err = beets.list(&Query::new()).unwrap_err();
 
         assert_eq!(err, Error::Invalid(invalid_string));
     }
@@ -544,7 +516,7 @@ mod tests {
             .return_once(|_| result);
 
         let mut beets = BeetsLibrary::new(executor);
-        let err = beets.list(&Query::default()).unwrap_err();
+        let err = beets.list(&Query::new()).unwrap_err();
 
         assert_eq!(err, Error::Invalid(invalid_string));
     }

src/library/mod.rs

@@ -1,6 +1,6 @@
 //! Module for interacting with the music library.
 
-use std::{fmt, num::ParseIntError, str::Utf8Error};
+use std::{collections::HashSet, fmt, num::ParseIntError, str::Utf8Error};
 
 #[cfg(test)]
 use mockall::automock;
@@ -9,81 +9,50 @@ use crate::Artist;
 
 pub mod beets;
 
-/// A single query option.
+/// Individual fields that can be queried on.
+#[derive(Debug, Hash, PartialEq, Eq)]
+pub enum Field {
+    AlbumArtist(String),
+    AlbumYear(u32),
+    AlbumTitle(String),
+    TrackNumber(u32),
+    TrackTitle(String),
+    TrackArtist(Vec<String>),
+    All(String),
+}
+
+/// A library query. Can include or exclude particular fields.
 #[derive(Debug, PartialEq, Eq)]
-pub enum QueryOption<T> {
-    /// Inclusive query.
-    Include(T),
-    /// Exclusive query.
-    Exclude(T),
-    /// No query.
-    None,
-}
-
-impl<T> Default for QueryOption<T> {
-    /// Create a [`QueryOption::None`] for type `T`.
-    fn default() -> Self {
-        Self::None
-    }
-}
-
-/// Options for refining library queries.
-#[derive(Debug, Default, PartialEq, Eq)]
 pub struct Query {
-    album_artist: QueryOption<String>,
+    include: HashSet<Field>,
-    album_year: QueryOption<u32>,
+    exclude: HashSet<Field>,
-    album_title: QueryOption<String>,
+}
-    track_number: QueryOption<u32>,
+
-    track_title: QueryOption<String>,
+impl Default for Query {
-    track_artist: QueryOption<Vec<String>>,
+    /// Create an empty query.
-    all: QueryOption<String>,
+    fn default() -> Self {
+        Self::new()
+    }
 }
 
 impl Query {
     /// Create an empty query.
     pub fn new() -> Self {
-        Query::default()
+        Query {
+            include: HashSet::new(),
+            exclude: HashSet::new(),
+        }
     }
 
-    /// Refine the query to a specific album artist.
+    /// Refine the query to include a particular search term.
-    pub fn album_artist(mut self, album_artist: QueryOption<String>) -> Self {
+    pub fn include(&mut self, field: Field) -> &mut Self {
-        self.album_artist = album_artist;
+        self.include.insert(field);
         self
     }
 
-    /// Refine the query to a specific album year.
+    /// Refine the query to exclude a particular search term.
-    pub fn album_year(mut self, album_year: QueryOption<u32>) -> Self {
+    pub fn exclude(&mut self, field: Field) -> &mut Self {
-        self.album_year = album_year;
+        self.exclude.insert(field);
-        self
-    }
-
-    /// Refine the query to a specific album title.
-    pub fn album_title(mut self, album_title: QueryOption<String>) -> Self {
-        self.album_title = album_title;
-        self
-    }
-
-    /// Refine the query to a specific track number.
-    pub fn track_number(mut self, track_number: QueryOption<u32>) -> Self {
-        self.track_number = track_number;
-        self
-    }
-
-    /// Refine the query to a specific track title.
-    pub fn track_title(mut self, track_title: QueryOption<String>) -> Self {
-        self.track_title = track_title;
-        self
-    }
-
-    /// Refine the query to a specific set of track artists.
-    pub fn track_artist(mut self, track_artist: QueryOption<Vec<String>>) -> Self {
-        self.track_artist = track_artist;
-        self
-    }
-
-    /// Refine the query for all fields.
-    pub fn all(mut self, all: QueryOption<String>) -> Self {
-        self.all = all;
         self
     }
 }
@@ -144,7 +113,23 @@ pub trait Library {
 mod tests {
     use std::io;
 
-    use super::Error;
+    use super::{Error, Field, Query};
+
+    #[test]
+    fn query() {
+        let mut lhs = Query::new();
+        let mut rhs = Query::new();
+        assert_eq!(
+            lhs.include(Field::AlbumArtist(String::from("some.artist")))
+                .exclude(Field::TrackTitle(String::from("some.title")))
+                .exclude(Field::TrackTitle(String::from("some.title")))
+                .include(Field::TrackNumber(6)),
+            rhs.exclude(Field::TrackTitle(String::from("some.title")))
+                .include(Field::TrackNumber(6))
+                .include(Field::AlbumArtist(String::from("some.artist")))
+                .include(Field::AlbumArtist(String::from("some.artist"))),
+        )
+    }
 
     #[test]
     fn errors() {

tests/library/beets.rs

@@ -9,7 +9,7 @@ use once_cell::sync::Lazy;
 use musichoard::{
     library::{
         beets::{BeetsLibrary, BeetsLibraryCommandExecutor},
-        Library, Query, QueryOption,
+        Field, Library, Query,
     },
     Artist,
 };
@@ -37,7 +37,7 @@ fn test_no_config_list() {
     let beets_arc = BEETS_EMPTY_CONFIG.clone();
     let beets = &mut beets_arc.lock().unwrap();
 
-    let output = beets.list(&Query::default()).unwrap();
+    let output = beets.list(&Query::new()).unwrap();
 
     let expected: Vec<Artist> = vec![];
     assert_eq!(output, expected);
@@ -49,7 +49,7 @@ fn test_invalid_config() {
         &PathBuf::from("./tests/files/library/config-does-not-exist.yml"),
     )));
 
-    let result = beets.list(&Query::default());
+    let result = beets.list(&Query::new());
     assert!(result.is_err());
     assert!(!result.unwrap_err().to_string().is_empty());
 }
@@ -59,7 +59,7 @@ fn test_full_list() {
     let beets_arc = BEETS_TEST_CONFIG.clone();
     let beets = &mut beets_arc.lock().unwrap();
 
-    let output = beets.list(&Query::default()).unwrap();
+    let output = beets.list(&Query::new()).unwrap();
 
     let expected: Vec<Artist> = COLLECTION.to_owned();
     assert_eq!(output, expected);
@@ -71,7 +71,7 @@ fn test_album_artist_query() {
     let beets = &mut beets_arc.lock().unwrap();
 
     let output = beets
-        .list(&Query::default().album_artist(QueryOption::Include(String::from("Аркона"))))
+        .list(Query::new().include(Field::AlbumArtist(String::from("Аркона"))))
         .unwrap();
 
     let expected: Vec<Artist> = COLLECTION[0..1].to_owned();
@@ -84,7 +84,7 @@ fn test_album_title_query() {
     let beets = &mut beets_arc.lock().unwrap();
 
     let output = beets
-        .list(&Query::default().album_title(QueryOption::Include(String::from("Slovo"))))
+        .list(&Query::new().include(Field::AlbumTitle(String::from("Slovo"))))
         .unwrap();
 
     let expected: Vec<Artist> = COLLECTION[0..1].to_owned();
@@ -97,7 +97,7 @@ fn test_exclude_query() {
     let beets = &mut beets_arc.lock().unwrap();
 
     let output = beets
-        .list(&Query::default().album_artist(QueryOption::Exclude(String::from("Аркона"))))
+        .list(&Query::new().exclude(Field::AlbumArtist(String::from("Аркона"))))
         .unwrap();
 
     let expected: Vec<Artist> = COLLECTION[1..].to_owned();