Provide a keyboard shortcut to sync all existing albums with MusicBrainz #167

Merged
wojtek merged 13 commits from 166---provide-a-keyboard-shortcut-to-sync-all-existing-albums-with-musicbrainz into main 2024-08-27 17:55:53 +02:00
7 changed files with 198 additions and 67 deletions
Showing only changes of commit a9b57258aa - Show all commits

View File

@ -2,15 +2,11 @@
use std::{fmt, num};
// TODO: #[cfg(test)]
// TODO: use mockall::automock;
use uuid::{self, Uuid};
use crate::collection::album::Album;
/// Trait for interacting with the MusicBrainz API.
// TODO: #[cfg_attr(test, automock)]
pub trait IMusicBrainz {
fn lookup_artist_release_groups(&mut self, mbid: &Mbid) -> Result<Vec<Album>, Error>;
fn search_release_group(

View File

@ -2,8 +2,6 @@
pub mod client;
use std::fmt;
use serde::{de::DeserializeOwned, Deserialize};
use url::form_urlencoded;
@ -44,15 +42,6 @@ impl From<ClientError> for Error {
}
}
impl fmt::Display for ClientError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
ClientError::Client(s) => write!(f, "the API client failed: {s}"),
ClientError::Status(u) => write!(f, "the API client failed with status: {u}"),
}
}
}
pub struct MusicBrainzApi<Mbc> {
client: Mbc,
}
@ -391,11 +380,6 @@ mod tests {
let mut client = MockIMusicBrainzApiClient::new();
let error = ClientError::Client(String::from("get rekt"));
assert!(!error.to_string().is_empty());
assert!(!format!("{error:?}").is_empty());
let error = ClientError::Status(404);
assert!(!error.to_string().is_empty());
assert!(!format!("{error:?}").is_empty());
client

View File

@ -16,6 +16,7 @@ use musichoard::{
executor::{ssh::BeetsLibrarySshExecutor, BeetsLibraryProcessExecutor},
BeetsLibrary,
},
musicbrainz::api::{client::MusicBrainzApiClient, MusicBrainzApi},
},
interface::{
database::{IDatabase, NullDatabase},
@ -26,6 +27,12 @@ use musichoard::{
use tui::{App, EventChannel, EventHandler, EventListener, Tui, Ui};
const MUSICHOARD_HTTP_USER_AGENT: &str = concat!(
"MusicHoard/",
env!("CARGO_PKG_VERSION"),
" ( musichoard@thenineworlds.net )"
);
#[derive(StructOpt)]
struct Opt {
#[structopt(flatten)]
@ -74,7 +81,11 @@ fn with<Database: IDatabase, Library: ILibrary>(builder: MusicHoardBuilder<Datab
let listener = EventListener::new(channel.sender());
let handler = EventHandler::new(channel.receiver());
let app = App::new(music_hoard);
let client = MusicBrainzApiClient::new(MUSICHOARD_HTTP_USER_AGENT)
.expect("failed to initialise HTTP client");
let api = Box::new(MusicBrainzApi::new(client));
let app = App::new(music_hoard, api);
let ui = Ui;
// Run the TUI application.

View File

@ -1,10 +1,6 @@
use std::{thread, time};
use musichoard::{
collection::musicbrainz::IMusicBrainzRef,
external::musicbrainz::api::{client::MusicBrainzApiClient, MusicBrainzApi},
interface::musicbrainz::IMusicBrainz,
};
use musichoard::collection::musicbrainz::IMusicBrainzRef;
use crate::tui::{
app::{
@ -13,7 +9,6 @@ use crate::tui::{
AppPublic, AppState, IAppInteractBrowse,
},
lib::IMusicHoard,
MUSICHOARD_TUI_HTTP_USER_AGENT,
};
pub struct AppBrowse;
@ -90,19 +85,7 @@ impl<MH: IMusicHoard> IAppInteractBrowse for AppMachine<MH, AppBrowse> {
AppMachine::search(self.inner, orig).into()
}
fn fetch_musicbrainz(self) -> Self::APP {
let client = match MusicBrainzApiClient::new(MUSICHOARD_TUI_HTTP_USER_AGENT) {
Ok(client) => client,
Err(err) => {
return AppMachine::error(
self.inner,
format!("cannot fetch: HTTP client init failed: {err}"),
)
.into()
}
};
let mut api = MusicBrainzApi::new(client);
fn fetch_musicbrainz(mut self) -> Self::APP {
let coll = self.inner.music_hoard.get_collection();
let artist = match self.inner.selection.state_artist(coll) {
Some(artist_state) => &coll[artist_state.index],
@ -125,7 +108,7 @@ impl<MH: IMusicHoard> IAppInteractBrowse for AppMachine<MH, AppBrowse> {
continue;
}
match api.search_release_group(arid, album) {
match self.inner.mb_api.search_release_group(arid, album) {
Ok(matches) => artist_album_matches.push(AppMatchesInfo {
matching: album.clone(),
matches,
@ -148,10 +131,17 @@ impl<MH: IMusicHoard> IAppInteractBrowse for AppMachine<MH, AppBrowse> {
#[cfg(test)]
mod tests {
use mockall::{predicate, Sequence};
use musichoard::collection::album::Album;
use crate::tui::{
app::{
machine::tests::{inner, music_hoard},
Category, IAppInteract,
machine::tests::{inner, inner_with_mb, music_hoard},
Category, IAppAccess, IAppInteract, IAppInteractMatches,
},
lib::external::musicbrainz::{
self,
api::{Match, Mbid, MockIMusicBrainz},
},
testmod::COLLECTION,
};
@ -228,6 +218,104 @@ mod tests {
app.unwrap_search();
}
#[test]
fn fetch_musicbrainz() {
let mut mb_api = Box::new(MockIMusicBrainz::new());
let arid: Mbid = "11111111-1111-1111-1111-111111111111".try_into().unwrap();
let album_1 = COLLECTION[1].albums[0].clone();
let album_4 = COLLECTION[1].albums[3].clone();
let album_match_1_1 = Match::new(100, album_1.clone());
let album_match_1_2 = Match::new(50, album_4.clone());
let album_match_4_1 = Match::new(100, album_4.clone());
let album_match_4_2 = Match::new(30, album_1.clone());
let matches_1 = vec![album_match_1_1.clone(), album_match_1_2.clone()];
let matches_4 = vec![album_match_4_1.clone(), album_match_4_2.clone()];
let result_1: Result<Vec<Match<Album>>, musicbrainz::api::Error> = Ok(matches_1.clone());
let result_4: Result<Vec<Match<Album>>, musicbrainz::api::Error> = Ok(matches_4.clone());
// Other albums have an MBID and so they will be skipped.
let mut seq = Sequence::new();
mb_api
.expect_search_release_group()
.with(predicate::eq(arid.clone()), predicate::eq(album_1.clone()))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _| result_1);
mb_api
.expect_search_release_group()
.with(predicate::eq(arid.clone()), predicate::eq(album_4.clone()))
.times(1)
.in_sequence(&mut seq)
.return_once(|_, _| result_4);
let browse = AppMachine::browse(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api));
// Use the second artist for this test.
let browse = browse.increment_selection(Delta::Line).unwrap_browse();
let mut app = browse.fetch_musicbrainz();
let public = app.get();
assert!(matches!(public.state, AppState::Matches(_)));
let public_matches = public.state.unwrap_matches();
assert_eq!(public_matches.matching, Some(&album_1));
assert_eq!(public_matches.matches, Some(matches_1.as_slice()));
let mut app = app.unwrap_matches().select();
let public = app.get();
assert!(matches!(public.state, AppState::Matches(_)));
let public_matches = public.state.unwrap_matches();
assert_eq!(public_matches.matching, Some(&album_4));
assert_eq!(public_matches.matches, Some(matches_4.as_slice()));
let app = app.unwrap_matches().select();
app.unwrap_browse();
}
#[test]
fn fetch_musicbrainz_no_artist() {
let browse = AppMachine::browse(inner(music_hoard(vec![])));
let app = browse.fetch_musicbrainz();
app.unwrap_error();
}
#[test]
fn fetch_musicbrainz_no_mbid() {
let browse = AppMachine::browse(inner(music_hoard(COLLECTION.to_owned())));
// Use the fourth artist for this test as they have no MBID.
let browse = browse.increment_selection(Delta::Line).unwrap_browse();
let browse = browse.increment_selection(Delta::Line).unwrap_browse();
let browse = browse.increment_selection(Delta::Line).unwrap_browse();
let app = browse.fetch_musicbrainz();
app.unwrap_error();
}
#[test]
fn fetch_musicbrainz_api_error() {
let mut mb_api = Box::new(MockIMusicBrainz::new());
let error = Err(musicbrainz::api::Error::RateLimit);
mb_api
.expect_search_release_group()
.times(1)
.return_once(|_, _| error);
let browse = AppMachine::browse(inner_with_mb(music_hoard(COLLECTION.to_owned()), mb_api));
let app = browse.fetch_musicbrainz();
app.unwrap_error();
}
#[test]
fn no_op() {
let browse = AppMachine::browse(inner(music_hoard(vec![])));

View File

@ -8,7 +8,7 @@ mod search;
use crate::tui::{
app::{selection::Selection, AppPublic, AppPublicInner, AppState, IAppAccess, IAppInteract},
lib::IMusicHoard,
lib::{external::musicbrainz::api::IMusicBrainz, IMusicHoard},
};
use browse::AppBrowse;
@ -37,13 +37,14 @@ pub struct AppMachine<MH: IMusicHoard, STATE> {
pub struct AppInner<MH: IMusicHoard> {
running: bool,
music_hoard: MH,
mb_api: Box<dyn IMusicBrainz>,
selection: Selection,
}
impl<MH: IMusicHoard> App<MH> {
pub fn new(mut music_hoard: MH) -> Self {
pub fn new(mut music_hoard: MH, mb_api: Box<dyn IMusicBrainz>) -> Self {
let init_result = Self::init(&mut music_hoard);
let inner = AppInner::new(music_hoard);
let inner = AppInner::new(music_hoard, mb_api);
match init_result {
Ok(()) => AppMachine::browse(inner).into(),
Err(err) => AppMachine::critical(inner, err.to_string()).into(),
@ -120,11 +121,12 @@ impl<MH: IMusicHoard> IAppAccess for App<MH> {
}
impl<MH: IMusicHoard> AppInner<MH> {
pub fn new(music_hoard: MH) -> Self {
pub fn new(music_hoard: MH, mb_api: Box<dyn IMusicBrainz>) -> Self {
let selection = Selection::new(music_hoard.get_collection());
AppInner {
running: true,
music_hoard,
mb_api,
selection,
}
}
@ -145,7 +147,7 @@ mod tests {
use crate::tui::{
app::{AppState, IAppInteract, IAppInteractBrowse},
lib::MockIMusicHoard,
lib::{external::musicbrainz::api::MockIMusicBrainz, MockIMusicHoard},
};
use super::*;
@ -219,13 +221,24 @@ mod tests {
music_hoard
}
fn mb_api() -> Box<MockIMusicBrainz> {
Box::new(MockIMusicBrainz::new())
}
pub fn inner(music_hoard: MockIMusicHoard) -> AppInner<MockIMusicHoard> {
AppInner::new(music_hoard)
AppInner::new(music_hoard, mb_api())
}
pub fn inner_with_mb(
music_hoard: MockIMusicHoard,
mb_api: Box<MockIMusicBrainz>,
) -> AppInner<MockIMusicHoard> {
AppInner::new(music_hoard, mb_api)
}
#[test]
fn state_browse() {
let mut app = App::new(music_hoard_init(vec![]));
let mut app = App::new(music_hoard_init(vec![]), mb_api());
assert!(app.is_running());
let state = app.state();
@ -241,7 +254,7 @@ mod tests {
#[test]
fn state_info() {
let mut app = App::new(music_hoard_init(vec![]));
let mut app = App::new(music_hoard_init(vec![]), mb_api());
assert!(app.is_running());
app = app.unwrap_browse().show_info_overlay();
@ -259,7 +272,7 @@ mod tests {
#[test]
fn state_reload() {
let mut app = App::new(music_hoard_init(vec![]));
let mut app = App::new(music_hoard_init(vec![]), mb_api());
assert!(app.is_running());
app = app.unwrap_browse().show_reload_menu();
@ -277,7 +290,7 @@ mod tests {
#[test]
fn state_search() {
let mut app = App::new(music_hoard_init(vec![]));
let mut app = App::new(music_hoard_init(vec![]), mb_api());
assert!(app.is_running());
app = app.unwrap_browse().begin_search();
@ -295,7 +308,7 @@ mod tests {
#[test]
fn state_matches() {
let mut app = App::new(music_hoard_init(vec![]));
let mut app = App::new(music_hoard_init(vec![]), mb_api());
assert!(app.is_running());
app = AppMachine::matches(app.unwrap_browse().inner, vec![]).into();
@ -313,7 +326,7 @@ mod tests {
#[test]
fn state_error() {
let mut app = App::new(music_hoard_init(vec![]));
let mut app = App::new(music_hoard_init(vec![]), mb_api());
assert!(app.is_running());
app = AppMachine::error(app.unwrap_browse().inner, "get rekt").into();
@ -331,7 +344,7 @@ mod tests {
#[test]
fn state_critical() {
let mut app = App::new(music_hoard_init(vec![]));
let mut app = App::new(music_hoard_init(vec![]), mb_api());
assert!(app.is_running());
app = AppMachine::critical(app.unwrap_browse().inner, "get rekt").into();
@ -357,7 +370,7 @@ mod tests {
.return_once(|| Err(musichoard::Error::LibraryError(String::from("get rekt"))));
music_hoard.expect_get_collection().return_const(vec![]);
let app = App::new(music_hoard);
let app = App::new(music_hoard, mb_api());
assert!(app.is_running());
app.unwrap_critical();
}

View File

@ -1,6 +1,6 @@
use musichoard::{
collection::Collection, interface::database::IDatabase, interface::library::ILibrary,
IMusicHoardBase, IMusicHoardDatabase, IMusicHoardLibrary, MusicHoard,
collection::Collection, interface, IMusicHoardBase, IMusicHoardDatabase, IMusicHoardLibrary,
MusicHoard,
};
#[cfg(test)]
@ -14,7 +14,9 @@ pub trait IMusicHoard {
}
// GRCOV_EXCL_START
impl<Database: IDatabase, Library: ILibrary> IMusicHoard for MusicHoard<Database, Library> {
impl<Database: interface::database::IDatabase, Library: interface::library::ILibrary> IMusicHoard
for MusicHoard<Database, Library>
{
fn rescan_library(&mut self) -> Result<(), musichoard::Error> {
<Self as IMusicHoardLibrary>::rescan_library(self)
}
@ -28,3 +30,45 @@ impl<Database: IDatabase, Library: ILibrary> IMusicHoard for MusicHoard<Database
}
}
// GRCOV_EXCL_STOP
pub mod external {
pub mod musicbrainz {
pub mod api {
use musichoard::{
collection::album::Album,
external::musicbrainz::api::{IMusicBrainzApiClient, MusicBrainzApi},
interface,
};
#[cfg(test)]
use mockall::automock;
pub type Match<T> = interface::musicbrainz::Match<T>;
pub type Mbid = interface::musicbrainz::Mbid;
pub type Error = interface::musicbrainz::Error;
#[cfg_attr(test, automock)]
pub trait IMusicBrainz {
fn search_release_group(
&mut self,
arid: &Mbid,
album: &Album,
) -> Result<Vec<Match<Album>>, Error>;
}
// GRCOV_EXCL_START
impl<Mbc: IMusicBrainzApiClient> IMusicBrainz for MusicBrainzApi<Mbc> {
fn search_release_group(
&mut self,
arid: &Mbid,
album: &Album,
) -> Result<Vec<Match<Album>>, Error> {
<Self as interface::musicbrainz::IMusicBrainz>::search_release_group(
self, arid, album,
)
}
}
// GRCOV_EXCL_STOP
}
}
}

View File

@ -26,12 +26,6 @@ use crate::tui::{
ui::IUi,
};
const MUSICHOARD_TUI_HTTP_USER_AGENT: &str = concat!(
"MusicHoard/",
env!("CARGO_PKG_VERSION"),
" ( musichoard@thenineworlds.net )"
);
#[derive(Debug, Eq, PartialEq)]
pub enum Error {
Io(String),
@ -179,6 +173,7 @@ mod testmod;
mod tests {
use std::{io, thread};
use lib::external::musicbrainz::api::MockIMusicBrainz;
use ratatui::{backend::TestBackend, Terminal};
use musichoard::collection::Collection;
@ -207,7 +202,7 @@ mod tests {
}
fn app(collection: Collection) -> App<MockIMusicHoard> {
App::new(music_hoard(collection))
App::new(music_hoard(collection), Box::new(MockIMusicBrainz::new()))
}
fn listener() -> MockIEventListener {