Streamline some code
All checks were successful
Cargo CI / Build and Test (pull_request) Successful in 3m9s
Cargo CI / Lint (pull_request) Successful in 1m16s

This commit is contained in:
Wojciech Kozlowski 2024-03-03 23:21:44 +01:00
parent 54646b1c03
commit 88d1dc2b01
8 changed files with 227 additions and 113 deletions

View File

@ -118,6 +118,12 @@ impl Merge for Album {
}
}
impl<S: Into<String>> From<S> for AlbumId {
fn from(value: S) -> Self {
AlbumId::new(value)
}
}
impl AsRef<AlbumId> for AlbumId {
fn as_ref(&self) -> &AlbumId {
self
@ -193,6 +199,32 @@ mod tests {
assert!(album_1 < album_2);
}
#[test]
fn set_clear_seq() {
let mut album = Album {
id: "an album".into(),
date: AlbumDate::default(),
seq: AlbumSeq::default(),
tracks: vec![],
};
assert_eq!(album.seq, AlbumSeq(0));
// Setting a seq on an album.
album.set_seq(AlbumSeq(6));
assert_eq!(album.seq, AlbumSeq(6));
album.set_seq(AlbumSeq(6));
assert_eq!(album.seq, AlbumSeq(6));
album.set_seq(AlbumSeq(8));
assert_eq!(album.seq, AlbumSeq(8));
// Clearing seq.
album.clear_seq();
assert_eq!(album.seq, AlbumSeq(0));
}
#[test]
fn merge_album_no_overlap() {
let left = FULL_COLLECTION[0].albums[0].to_owned();

View File

@ -2,6 +2,7 @@ use std::{
collections::HashMap,
fmt::{self, Debug, Display},
mem,
str::FromStr,
};
use url::Url;
@ -139,6 +140,12 @@ impl Merge for Artist {
}
}
impl<S: Into<String>> From<S> for ArtistId {
fn from(value: S) -> Self {
ArtistId::new(value)
}
}
impl AsRef<ArtistId> for ArtistId {
fn as_ref(&self) -> &ArtistId {
self
@ -169,9 +176,13 @@ pub struct MusicBrainz(Url);
impl MusicBrainz {
/// Validate and wrap a MusicBrainz URL.
pub fn new<S: AsRef<str>>(url: S) -> Result<Self, Error> {
pub fn new_from_str<S: AsRef<str>>(url: S) -> Result<Self, Error> {
let url = Url::parse(url.as_ref())?;
Self::new_from_url(url)
}
/// Validate and wrap a MusicBrainz URL.
pub fn new_from_url(url: Url) -> Result<Self, Error> {
if !url
.domain()
.map(|u| u.ends_with("musicbrainz.org"))
@ -199,11 +210,36 @@ impl AsRef<str> for MusicBrainz {
}
}
impl TryFrom<&str> for MusicBrainz {
impl FromStr for MusicBrainz {
type Err = Error;
fn from_str(s: &str) -> Result<Self, Self::Err> {
MusicBrainz::new_from_str(s)
}
}
// A blanket TryFrom would be better, but https://stackoverflow.com/a/64407892
macro_rules! impl_try_from_for_musicbrainz {
($from:ty) => {
impl TryFrom<$from> for MusicBrainz {
type Error = Error;
fn try_from(value: &str) -> Result<Self, Self::Error> {
MusicBrainz::new(value)
fn try_from(value: $from) -> Result<Self, Self::Error> {
MusicBrainz::new_from_str(value)
}
}
};
}
impl_try_from_for_musicbrainz!(&str);
impl_try_from_for_musicbrainz!(&String);
impl_try_from_for_musicbrainz!(String);
impl TryFrom<Url> for MusicBrainz {
type Error = Error;
fn try_from(value: Url) -> Result<Self, Self::Error> {
MusicBrainz::new_from_url(value)
}
}
@ -230,34 +266,35 @@ mod tests {
#[test]
fn musicbrainz() {
let uuid = "d368baa8-21ca-4759-9731-0b2753071ad8";
let url = format!("https://musicbrainz.org/artist/{uuid}");
let mb = MusicBrainz::new(&url).unwrap();
assert_eq!(url, mb.as_ref());
let url_str = format!("https://musicbrainz.org/artist/{uuid}");
let url: Url = url_str.as_str().try_into().unwrap();
let mb: MusicBrainz = url.try_into().unwrap();
assert_eq!(url_str, mb.as_ref());
assert_eq!(uuid, mb.mbid());
let url = "not a url at all".to_string();
let expected_error: Error = url::ParseError::RelativeUrlWithoutBase.into();
let actual_error = MusicBrainz::new(url).unwrap_err();
let actual_error = MusicBrainz::from_str(&url).unwrap_err();
assert_eq!(actual_error, expected_error);
assert_eq!(actual_error.to_string(), expected_error.to_string());
let url = "https://musicbrainz.org/artist/i-am-not-a-uuid".to_string();
let expected_error: Error = Uuid::try_parse("i-am-not-a-uuid").unwrap_err().into();
let actual_error = MusicBrainz::new(url).unwrap_err();
let actual_error = MusicBrainz::from_str(&url).unwrap_err();
assert_eq!(actual_error, expected_error);
assert_eq!(actual_error.to_string(), expected_error.to_string());
let url = "https://musicbrainz.org/artist".to_string();
let expected_error = Error::UrlError(format!("invalid MusicBrainz URL: {url}"));
let actual_error = MusicBrainz::new(&url).unwrap_err();
let actual_error = MusicBrainz::from_str(&url).unwrap_err();
assert_eq!(actual_error, expected_error);
assert_eq!(actual_error.to_string(), expected_error.to_string());
}
#[test]
fn urls() {
assert!(MusicBrainz::new(MUSICBRAINZ).is_ok());
assert!(MusicBrainz::new(MUSICBUTLER).is_err());
assert!(MusicBrainz::from_str(MUSICBRAINZ).is_ok());
assert!(MusicBrainz::from_str(MUSICBUTLER).is_err());
}
#[test]
@ -266,7 +303,7 @@ mod tests {
let sort_id_1 = ArtistId::new("sort id 1");
let sort_id_2 = ArtistId::new("sort id 2");
let mut artist = Artist::new(artist_id.clone());
let mut artist = Artist::new(&artist_id.name);
assert_eq!(artist.id, artist_id);
assert_eq!(artist.sort, None);
@ -317,14 +354,14 @@ mod tests {
// Setting a URL on an artist.
artist.set_musicbrainz_url(MUSICBRAINZ.try_into().unwrap());
_ = expected.insert(MusicBrainz::new(MUSICBRAINZ).unwrap());
_ = expected.insert(MUSICBRAINZ.try_into().unwrap());
assert_eq!(artist.musicbrainz, expected);
artist.set_musicbrainz_url(MUSICBRAINZ.try_into().unwrap());
assert_eq!(artist.musicbrainz, expected);
artist.set_musicbrainz_url(MUSICBRAINZ_2.try_into().unwrap());
_ = expected.insert(MusicBrainz::new(MUSICBRAINZ_2).unwrap());
_ = expected.insert(MUSICBRAINZ_2.try_into().unwrap());
assert_eq!(artist.musicbrainz, expected);
// Clearing URLs.

View File

@ -5,7 +5,7 @@ use serde::Deserialize;
use crate::core::{
collection::{
album::{Album, AlbumDate, AlbumId, AlbumSeq},
artist::{Artist, ArtistId, MusicBrainz},
artist::{Artist, ArtistId},
Collection,
},
database::LoadError,
@ -51,7 +51,7 @@ impl TryFrom<DeserializeArtist> for Artist {
Ok(Artist {
id: ArtistId::new(artist.name),
sort: artist.sort.map(ArtistId::new),
musicbrainz: artist.musicbrainz.map(MusicBrainz::new).transpose()?,
musicbrainz: artist.musicbrainz.map(TryInto::try_into).transpose()?,
properties: artist.properties,
albums: artist.albums.into_iter().map(Into::into).collect(),
})

View File

@ -3,7 +3,7 @@ use std::collections::HashMap;
use crate::core::{
collection::{
album::{Album, AlbumDate, AlbumId, AlbumSeq},
artist::{Artist, ArtistId},
artist::{Artist, ArtistId, MusicBrainz},
track::{Track, TrackId, TrackNum, TrackQuality},
Collection, MergeCollections,
},
@ -259,63 +259,61 @@ impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
Ok(())
}
fn update_collection<FN>(&mut self, func: FN) -> Result<(), Error>
fn update_collection<FnColl>(&mut self, fn_coll: FnColl) -> Result<(), Error>
where
FN: FnOnce(&mut Collection),
FnColl: FnOnce(&mut Collection),
{
func(&mut self.pre_commit);
fn_coll(&mut self.pre_commit);
self.commit()
}
fn update_artist_and<ID: AsRef<ArtistId>, FNARTIST, FNCOLL>(
fn update_artist_and<FnArtist, FnColl>(
&mut self,
artist_id: ID,
fn_artist: FNARTIST,
fn_collection: FNCOLL,
artist_id: &ArtistId,
fn_artist: FnArtist,
fn_coll: FnColl,
) -> Result<(), Error>
where
FNARTIST: FnOnce(&mut Artist),
FNCOLL: FnOnce(&mut Collection),
FnArtist: FnOnce(&mut Artist),
FnColl: FnOnce(&mut Collection),
{
fn_artist(Self::get_artist_mut_or_err(
&mut self.pre_commit,
artist_id.as_ref(),
)?);
self.update_collection(fn_collection)
let artist = Self::get_artist_mut_or_err(&mut self.pre_commit, artist_id)?;
fn_artist(artist);
self.update_collection(fn_coll)
}
fn update_artist<ID: AsRef<ArtistId>, FN>(
fn update_artist<FnArtist>(
&mut self,
artist_id: ID,
func: FN,
artist_id: &ArtistId,
fn_artist: FnArtist,
) -> Result<(), Error>
where
FN: FnOnce(&mut Artist),
FnArtist: FnOnce(&mut Artist),
{
self.update_artist_and(artist_id, func, |_| {})
self.update_artist_and(artist_id, fn_artist, |_| {})
}
fn update_album_and<ARTIST: AsRef<ArtistId>, ALBUM: AsRef<AlbumId>, FNALBUM, FNARTIST, FNCOLL>(
fn update_album_and<FnAlbum, FnArtist, FnColl>(
&mut self,
artist_id: ARTIST,
album_id: ALBUM,
fn_album: FNALBUM,
fn_artist: FNARTIST,
fn_collection: FNCOLL,
artist_id: &ArtistId,
album_id: &AlbumId,
fn_album: FnAlbum,
fn_artist: FnArtist,
fn_coll: FnColl,
) -> Result<(), Error>
where
FNALBUM: FnOnce(&mut Album),
FNARTIST: FnOnce(&mut Artist),
FNCOLL: FnOnce(&mut Collection),
FnAlbum: FnOnce(&mut Album),
FnArtist: FnOnce(&mut Artist),
FnColl: FnOnce(&mut Collection),
{
let artist = Self::get_artist_mut_or_err(&mut self.pre_commit, artist_id.as_ref())?;
let album = Self::get_album_mut_or_err(artist, album_id.as_ref())?;
let artist = Self::get_artist_mut_or_err(&mut self.pre_commit, artist_id)?;
let album = Self::get_album_mut_or_err(artist, album_id)?;
fn_album(album);
fn_artist(artist);
self.update_collection(fn_collection)
self.update_collection(fn_coll)
}
pub fn add_artist<ID: Into<ArtistId>>(&mut self, artist_id: ID) -> Result<(), Error> {
pub fn add_artist<IntoId: Into<ArtistId>>(&mut self, artist_id: IntoId) -> Result<(), Error> {
let artist_id: ArtistId = artist_id.into();
self.update_collection(|collection| {
@ -326,7 +324,7 @@ impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
})
}
pub fn remove_artist<ID: AsRef<ArtistId>>(&mut self, artist_id: ID) -> Result<(), Error> {
pub fn remove_artist<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error> {
self.update_collection(|collection| {
let index_opt = collection.iter().position(|a| &a.id == artist_id.as_ref());
if let Some(index) = index_opt {
@ -335,102 +333,109 @@ impl<LIB, DB: IDatabase> MusicHoard<LIB, DB> {
})
}
pub fn set_artist_sort<ID: AsRef<ArtistId>, SORT: Into<ArtistId>>(
pub fn set_artist_sort<Id: AsRef<ArtistId>, IntoId: Into<ArtistId>>(
&mut self,
artist_id: ID,
artist_sort: SORT,
artist_id: Id,
artist_sort: IntoId,
) -> Result<(), Error> {
self.update_artist_and(
artist_id,
artist_id.as_ref(),
|artist| artist.set_sort_key(artist_sort),
|collection| Self::sort_artists(collection),
)
}
pub fn clear_artist_sort<ID: AsRef<ArtistId>>(&mut self, artist_id: ID) -> Result<(), Error> {
pub fn clear_artist_sort<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error> {
self.update_artist_and(
artist_id,
artist_id.as_ref(),
|artist| artist.clear_sort_key(),
|collection| Self::sort_artists(collection),
)
}
pub fn set_artist_musicbrainz<ID: AsRef<ArtistId>, S: AsRef<str>>(
pub fn set_artist_musicbrainz<Id: AsRef<ArtistId>, Mb: TryInto<MusicBrainz, Error = E>, E>(
&mut self,
artist_id: ID,
url: S,
) -> Result<(), Error> {
let url = url.as_ref().try_into()?;
self.update_artist(artist_id, |artist| artist.set_musicbrainz_url(url))
artist_id: Id,
url: Mb,
) -> Result<(), Error>
where
Error: From<E>,
{
let mb = url.try_into()?;
self.update_artist(artist_id.as_ref(), |artist| artist.set_musicbrainz_url(mb))
}
pub fn clear_artist_musicbrainz<ID: AsRef<ArtistId>>(
pub fn clear_artist_musicbrainz<Id: AsRef<ArtistId>>(
&mut self,
artist_id: ID,
artist_id: Id,
) -> Result<(), Error> {
self.update_artist(artist_id, |artist| artist.clear_musicbrainz_url())
self.update_artist(artist_id.as_ref(), |artist| artist.clear_musicbrainz_url())
}
pub fn add_to_artist_property<ID: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
pub fn add_to_artist_property<Id: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
&mut self,
artist_id: ID,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error> {
self.update_artist(artist_id, |artist| artist.add_to_property(property, values))
self.update_artist(artist_id.as_ref(), |artist| {
artist.add_to_property(property, values)
})
}
pub fn remove_from_artist_property<ID: AsRef<ArtistId>, S: AsRef<str>>(
pub fn remove_from_artist_property<Id: AsRef<ArtistId>, S: AsRef<str>>(
&mut self,
artist_id: ID,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error> {
self.update_artist(artist_id, |artist| {
self.update_artist(artist_id.as_ref(), |artist| {
artist.remove_from_property(property, values)
})
}
pub fn set_artist_property<ID: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
pub fn set_artist_property<Id: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
&mut self,
artist_id: ID,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error> {
self.update_artist(artist_id, |artist| artist.set_property(property, values))
self.update_artist(artist_id.as_ref(), |artist| {
artist.set_property(property, values)
})
}
pub fn clear_artist_property<ID: AsRef<ArtistId>, S: AsRef<str>>(
pub fn clear_artist_property<Id: AsRef<ArtistId>, S: AsRef<str>>(
&mut self,
artist_id: ID,
artist_id: Id,
property: S,
) -> Result<(), Error> {
self.update_artist(artist_id, |artist| artist.clear_property(property))
self.update_artist(artist_id.as_ref(), |artist| artist.clear_property(property))
}
pub fn set_album_seq<ARTIST: AsRef<ArtistId>, ALBUM: AsRef<AlbumId>>(
pub fn set_album_seq<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self,
artist_id: ARTIST,
album_id: ALBUM,
artist_id: ArtistIdRef,
album_id: AlbumIdRef,
seq: u8,
) -> Result<(), Error> {
self.update_album_and(
artist_id,
album_id,
artist_id.as_ref(),
album_id.as_ref(),
|album| album.set_seq(AlbumSeq(seq)),
|artist| artist.albums.sort_unstable(),
|_| {},
)
}
pub fn clear_album_seq<ARTIST: AsRef<ArtistId>, ALBUM: AsRef<AlbumId>>(
pub fn clear_album_seq<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self,
artist_id: ARTIST,
album_id: ALBUM,
artist_id: ArtistIdRef,
album_id: AlbumIdRef,
) -> Result<(), Error> {
self.update_album_and(
artist_id,
album_id,
artist_id.as_ref(),
album_id.as_ref(),
|album| album.clear_seq(),
|artist| artist.albums.sort_unstable(),
|_| {},
@ -621,7 +626,7 @@ mod tests {
assert!(music_hoard
.set_artist_musicbrainz(&artist_id, MUSICBRAINZ)
.is_ok());
_ = expected.insert(MusicBrainz::new(MUSICBRAINZ).unwrap());
_ = expected.insert(MUSICBRAINZ.try_into().unwrap());
assert_eq!(music_hoard.collection[0].musicbrainz, expected);
// Clearing URLs on an artist that does not exist is an error.
@ -733,6 +738,51 @@ mod tests {
assert!(music_hoard.collection[0].properties.is_empty());
}
#[test]
fn set_clear_album_seq() {
let mut database = MockIDatabase::new();
let artist_id = ArtistId::new("an artist");
let album_id = AlbumId::new("an album");
let album_id_2 = AlbumId::new("another album");
let mut database_result = vec![Artist::new(artist_id.clone())];
database_result[0].albums.push(Album {
id: album_id.clone(),
date: AlbumDate::default(),
seq: AlbumSeq::default(),
tracks: vec![],
});
database
.expect_load()
.times(1)
.return_once(|| Ok(database_result));
database.expect_save().times(2).returning(|_| Ok(()));
let mut music_hoard = MusicHoard::database(database).unwrap();
assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(0));
// Seting seq on an album not belonging to the artist is an error.
assert!(music_hoard
.set_album_seq(&artist_id, &album_id_2, 6)
.is_err());
assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(0));
// Set seq.
assert!(music_hoard.set_album_seq(&artist_id, &album_id, 6).is_ok());
assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(6));
// Clearing seq on an album that does not exist is an error.
assert!(music_hoard
.clear_album_seq(&artist_id, &album_id_2)
.is_err());
// Clear seq.
assert!(music_hoard.clear_album_seq(&artist_id, &album_id).is_ok());
assert_eq!(music_hoard.collection[0].albums[0].seq, AlbumSeq(0));
}
#[test]
fn merge_collection_no_overlap() {
let half: usize = FULL_COLLECTION.len() / 2;

View File

@ -1,5 +1,5 @@
use once_cell::sync::Lazy;
use std::collections::HashMap;
use std::{collections::HashMap, str::FromStr};
use crate::core::collection::{
album::{Album, AlbumDate, AlbumId, AlbumMonth, AlbumSeq},

View File

@ -454,11 +454,9 @@ macro_rules! full_collection {
let artist_a = iter.next().unwrap();
assert_eq!(artist_a.id.name, "Album_Artist A");
artist_a.musicbrainz = Some(
MusicBrainz::new(
artist_a.musicbrainz = Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000",
).unwrap(),
);
).unwrap());
artist_a.properties = HashMap::from([
(String::from("MusicButler"), vec![
@ -477,11 +475,9 @@ macro_rules! full_collection {
let artist_b = iter.next().unwrap();
assert_eq!(artist_b.id.name, "Album_Artist B");
artist_b.musicbrainz = Some(
MusicBrainz::new(
artist_b.musicbrainz = Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111",
).unwrap(),
);
).unwrap());
artist_b.properties = HashMap::from([
(String::from("MusicButler"), vec![
@ -504,11 +500,9 @@ macro_rules! full_collection {
let artist_c = iter.next().unwrap();
assert_eq!(artist_c.id.name, "The Album_Artist C");
artist_c.musicbrainz = Some(
MusicBrainz::new(
artist_c.musicbrainz = Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111",
).unwrap(),
);
).unwrap());
// Nothing for artist_d

View File

@ -1,10 +1,11 @@
use std::{collections::HashMap, str::FromStr};
use musichoard::collection::{
album::{Album, AlbumDate, AlbumId, AlbumMonth, AlbumSeq},
artist::{Artist, ArtistId, MusicBrainz},
track::{Track, TrackFormat, TrackId, TrackNum, TrackQuality},
};
use once_cell::sync::Lazy;
use std::collections::HashMap;
use crate::tests::*;

View File

@ -1,5 +1,5 @@
use once_cell::sync::Lazy;
use std::collections::HashMap;
use std::{collections::HashMap, str::FromStr};
use musichoard::collection::{
album::{Album, AlbumDate, AlbumId, AlbumMonth, AlbumSeq},
@ -17,8 +17,8 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
sort: Some(ArtistId{
name: String::from("Arkona")
}),
musicbrainz: Some(MusicBrainz::new(
"https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212",
musicbrainz: Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/baad262d-55ef-427a-83c7-f7530964f212"
).unwrap()),
properties: HashMap::from([
(String::from("MusicButler"), vec![
@ -204,7 +204,7 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
name: String::from("Eluveitie"),
},
sort: None,
musicbrainz: Some(MusicBrainz::new(
musicbrainz: Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/8000598a-5edb-401c-8e6d-36b167feaf38",
).unwrap()),
properties: HashMap::from([
@ -447,7 +447,7 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
name: String::from("Frontside"),
},
sort: None,
musicbrainz: Some(MusicBrainz::new(
musicbrainz: Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/3a901353-fccd-4afd-ad01-9c03f451b490",
).unwrap()),
properties: HashMap::from([
@ -600,7 +600,7 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
sort: Some(ArtistId {
name: String::from("Heavens Basement"),
}),
musicbrainz: Some(MusicBrainz::new(
musicbrainz: Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/c2c4d56a-d599-4a18-bd2f-ae644e2198cc",
).unwrap()),
properties: HashMap::from([
@ -730,7 +730,7 @@ pub static COLLECTION: Lazy<Vec<Artist>> = Lazy::new(|| -> Collection {
name: String::from("Metallica"),
},
sort: None,
musicbrainz: Some(MusicBrainz::new(
musicbrainz: Some(MusicBrainz::from_str(
"https://musicbrainz.org/artist/65f4f0c5-ef9e-490c-aee3-909e7ae6b2ab",
).unwrap()),
properties: HashMap::from([