WIP: Refactor the IDatabase calls to write directly to the database #271

Draft
wojtek wants to merge 12 commits from 268---refactor-the-idatabase-calls-to-write-directly-to-the-database into main
2 changed files with 13 additions and 401 deletions
Showing only changes of commit c2973680e8 - Show all commits

View File

@ -37,7 +37,6 @@ pub trait IMusicHoardBasePrivate {
fn filter_collection(&self) -> Collection; fn filter_collection(&self) -> Collection;
fn filter_artist(&self, artist: &Artist) -> Option<Artist>; fn filter_artist(&self, artist: &Artist) -> Option<Artist>;
fn get_artist<'a>(collection: &'a Collection, artist_id: &ArtistId) -> Option<&'a Artist>;
fn get_artist_mut<'a>( fn get_artist_mut<'a>(
collection: &'a mut Collection, collection: &'a mut Collection,
artist_id: &ArtistId, artist_id: &ArtistId,
@ -105,10 +104,6 @@ impl<Database, Library> IMusicHoardBasePrivate for MusicHoard<Database, Library>
Some(Artist { meta, albums }) 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)
}
fn get_artist_mut<'a>( fn get_artist_mut<'a>(
collection: &'a mut Collection, collection: &'a mut Collection,
artist_id: &ArtistId, artist_id: &ArtistId,

View File

@ -8,7 +8,7 @@ use crate::{
}, },
core::{ core::{
collection::{ collection::{
album::{Album, AlbumId, AlbumSeq}, album::{Album, AlbumId},
artist::{Artist, ArtistId}, artist::{Artist, ArtistId},
Collection, Collection,
}, },
@ -20,9 +20,6 @@ use crate::{
pub trait IMusicHoardDatabase { pub trait IMusicHoardDatabase {
fn reload_database(&mut self) -> Result<(), Error>; fn reload_database(&mut self) -> Result<(), Error>;
fn add_artist<IntoId: Into<ArtistId>>(&mut self, artist_id: IntoId) -> Result<(), Error>;
fn remove_artist<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>;
fn set_artist_mb_ref<Id: AsRef<ArtistId>>( fn set_artist_mb_ref<Id: AsRef<ArtistId>>(
&mut self, &mut self,
artist_id: Id, artist_id: Id,
@ -30,13 +27,6 @@ pub trait IMusicHoardDatabase {
) -> Result<(), Error>; ) -> Result<(), Error>;
fn clear_artist_mb_ref<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>; fn clear_artist_mb_ref<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>;
fn set_artist_sort<Id: AsRef<ArtistId>, S: Into<String>>(
&mut self,
artist_id: Id,
artist_sort: S,
) -> Result<(), Error>;
fn clear_artist_sort<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>;
fn merge_artist_info<Id: AsRef<ArtistId>>( fn merge_artist_info<Id: AsRef<ArtistId>>(
&mut self, &mut self,
artist_id: Id, artist_id: Id,
@ -44,30 +34,6 @@ pub trait IMusicHoardDatabase {
) -> Result<(), Error>; ) -> Result<(), Error>;
fn clear_artist_info<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>; fn clear_artist_info<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error>;
fn add_to_artist_property<Id: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
&mut self,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error>;
fn remove_from_artist_property<Id: AsRef<ArtistId>, S: AsRef<str>>(
&mut self,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error>;
fn set_artist_property<Id: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
&mut self,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error>;
fn clear_artist_property<Id: AsRef<ArtistId>, S: AsRef<str>>(
&mut self,
artist_id: Id,
property: S,
) -> Result<(), Error>;
fn add_album<ArtistIdRef: AsRef<ArtistId>>( fn add_album<ArtistIdRef: AsRef<ArtistId>>(
&mut self, &mut self,
artist_id: ArtistIdRef, artist_id: ArtistIdRef,
@ -90,17 +56,7 @@ pub trait IMusicHoardDatabase {
artist_id: ArtistIdRef, artist_id: ArtistIdRef,
album_id: AlbumIdRef, album_id: AlbumIdRef,
) -> Result<(), Error>; ) -> Result<(), Error>;
fn set_album_seq<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self,
artist_id: ArtistIdRef,
album_id: AlbumIdRef,
seq: u8,
) -> Result<(), Error>;
fn clear_album_seq<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self,
artist_id: ArtistIdRef,
album_id: AlbumIdRef,
) -> Result<(), Error>;
fn merge_album_info<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>( fn merge_album_info<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self, &mut self,
artist_id: ArtistIdRef, artist_id: ArtistIdRef,
@ -125,28 +81,6 @@ impl<Database: IDatabase, Library> IMusicHoardDatabase for MusicHoard<Database,
Ok(()) Ok(())
} }
fn add_artist<IntoId: Into<ArtistId>>(&mut self, artist_id: IntoId) -> Result<(), Error> {
let artist_id: ArtistId = artist_id.into();
self.update_collection(|collection| {
if Self::get_artist(collection, &artist_id).is_none() {
collection.push(Artist::new(artist_id));
Self::sort_artists(collection);
}
})
}
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.meta.id == artist_id.as_ref());
if let Some(index) = index_opt {
collection.remove(index);
}
})
}
fn set_artist_mb_ref<Id: AsRef<ArtistId>>( fn set_artist_mb_ref<Id: AsRef<ArtistId>>(
&mut self, &mut self,
artist_id: Id, artist_id: Id,
@ -167,26 +101,6 @@ impl<Database: IDatabase, Library> IMusicHoardDatabase for MusicHoard<Database,
) )
} }
fn set_artist_sort<Id: AsRef<ArtistId>, S: Into<String>>(
&mut self,
artist_id: Id,
artist_sort: S,
) -> Result<(), Error> {
self.update_artist_and(
artist_id.as_ref(),
|artist| artist.meta.set_sort_key(artist_sort),
|collection| Self::sort_artists(collection),
)
}
fn clear_artist_sort<Id: AsRef<ArtistId>>(&mut self, artist_id: Id) -> Result<(), Error> {
self.update_artist_and(
artist_id.as_ref(),
|artist| artist.meta.clear_sort_key(),
|collection| Self::sort_artists(collection),
)
}
fn merge_artist_info<Id: AsRef<ArtistId>>( fn merge_artist_info<Id: AsRef<ArtistId>>(
&mut self, &mut self,
artist_id: Id, artist_id: Id,
@ -204,49 +118,6 @@ impl<Database: IDatabase, Library> IMusicHoardDatabase for MusicHoard<Database,
}) })
} }
fn add_to_artist_property<Id: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
&mut self,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error> {
self.update_artist(artist_id.as_ref(), |artist| {
artist.meta.info.add_to_property(property, values)
})
}
fn remove_from_artist_property<Id: AsRef<ArtistId>, S: AsRef<str>>(
&mut self,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error> {
self.update_artist(artist_id.as_ref(), |artist| {
artist.meta.info.remove_from_property(property, values)
})
}
fn set_artist_property<Id: AsRef<ArtistId>, S: AsRef<str> + Into<String>>(
&mut self,
artist_id: Id,
property: S,
values: Vec<S>,
) -> Result<(), Error> {
self.update_artist(artist_id.as_ref(), |artist| {
artist.meta.info.set_property(property, values)
})
}
fn clear_artist_property<Id: AsRef<ArtistId>, S: AsRef<str>>(
&mut self,
artist_id: Id,
property: S,
) -> Result<(), Error> {
self.update_artist(artist_id.as_ref(), |artist| {
artist.meta.info.clear_property(property)
})
}
fn add_album<ArtistIdRef: AsRef<ArtistId>>( fn add_album<ArtistIdRef: AsRef<ArtistId>>(
&mut self, &mut self,
artist_id: ArtistIdRef, artist_id: ArtistIdRef,
@ -307,33 +178,6 @@ impl<Database: IDatabase, Library> IMusicHoardDatabase for MusicHoard<Database,
) )
} }
fn set_album_seq<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self,
artist_id: ArtistIdRef,
album_id: AlbumIdRef,
seq: u8,
) -> Result<(), Error> {
self.update_album_and(
artist_id.as_ref(),
album_id.as_ref(),
|album| album.meta.set_seq(AlbumSeq(seq)),
|artist| artist.albums.sort_unstable(),
)
}
fn clear_album_seq<ArtistIdRef: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self,
artist_id: ArtistIdRef,
album_id: AlbumIdRef,
) -> Result<(), Error> {
self.update_album_and(
artist_id.as_ref(),
album_id.as_ref(),
|album| album.meta.clear_seq(),
|artist| artist.albums.sort_unstable(),
)
}
fn merge_album_info<Id: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>( fn merge_album_info<Id: AsRef<ArtistId>, AlbumIdRef: AsRef<AlbumId>>(
&mut self, &mut self,
artist_id: Id, artist_id: Id,
@ -457,7 +301,7 @@ mod tests {
core::{ core::{
collection::artist::ArtistId, collection::artist::ArtistId,
interface::database::{self, MockIDatabase}, interface::database::{self, MockIDatabase},
musichoard::{base::IMusicHoardBase, NoLibrary}, musichoard::base::IMusicHoardBase,
testmod::FULL_COLLECTION, testmod::FULL_COLLECTION,
}, },
}; };
@ -465,106 +309,6 @@ mod tests {
use super::*; use super::*;
static MBID: &str = "d368baa8-21ca-4759-9731-0b2753071ad8"; static MBID: &str = "d368baa8-21ca-4759-9731-0b2753071ad8";
static MUSICBUTLER: &str = "https://www.musicbutler.io/artist-page/483340948";
static MUSICBUTLER_2: &str = "https://www.musicbutler.io/artist-page/658903042/";
#[test]
fn artist_new_delete() {
let artist_id = ArtistId::new("an artist");
let artist_id_2 = ArtistId::new("another artist");
let collection = FULL_COLLECTION.to_owned();
let mut with_artist = collection.clone();
with_artist.push(Artist::new(artist_id.clone()));
let mut database = MockIDatabase::new();
let mut seq = Sequence::new();
database
.expect_load()
.times(1)
.times(1)
.in_sequence(&mut seq)
.returning(|| Ok(FULL_COLLECTION.to_owned()));
database
.expect_save()
.times(3)
.in_sequence(&mut seq)
.with(predicate::eq(with_artist.clone()))
.returning(|_| Ok(()));
database
.expect_save()
.times(1)
.in_sequence(&mut seq)
.with(predicate::eq(collection.clone()))
.returning(|_| Ok(()));
let mut music_hoard = MusicHoard::database(database);
music_hoard.reload_database().unwrap();
assert_eq!(music_hoard.collection, collection);
assert!(music_hoard.add_artist(artist_id.clone()).is_ok());
assert_eq!(music_hoard.collection, with_artist);
assert!(music_hoard.add_artist(artist_id.clone()).is_ok());
assert_eq!(music_hoard.collection, with_artist);
assert!(music_hoard.remove_artist(&artist_id_2).is_ok());
assert_eq!(music_hoard.collection, with_artist);
assert!(music_hoard.remove_artist(&artist_id).is_ok());
assert_eq!(music_hoard.collection, collection);
}
#[test]
fn artist_sort_set_clear() {
let mut database = MockIDatabase::new();
database.expect_save().times(4).returning(|_| Ok(()));
type MH = MusicHoard<MockIDatabase, NoLibrary>;
let mut music_hoard: MH = MusicHoard::database(database);
let artist_1_id = ArtistId::new("the artist");
let artist_1_sort = String::from("artist, the");
// Must be after "artist, the", but before "the artist"
let artist_2_id = ArtistId::new("b-artist");
assert!(artist_1_sort < artist_2_id.name);
assert!(artist_2_id < artist_1_id);
assert!(music_hoard.add_artist(artist_1_id.clone()).is_ok());
assert!(music_hoard.add_artist(artist_2_id.clone()).is_ok());
let artist_1: &Artist = MH::get_artist(&music_hoard.collection, &artist_1_id).unwrap();
let artist_2: &Artist = MH::get_artist(&music_hoard.collection, &artist_2_id).unwrap();
assert!(artist_2 < artist_1);
assert_eq!(artist_1, &music_hoard.collection[1]);
assert_eq!(artist_2, &music_hoard.collection[0]);
music_hoard
.set_artist_sort(artist_1_id.as_ref(), artist_1_sort.clone())
.unwrap();
let artist_1: &Artist = MH::get_artist(&music_hoard.collection, &artist_1_id).unwrap();
let artist_2: &Artist = MH::get_artist(&music_hoard.collection, &artist_2_id).unwrap();
assert!(artist_1 < artist_2);
assert_eq!(artist_1, &music_hoard.collection[0]);
assert_eq!(artist_2, &music_hoard.collection[1]);
music_hoard.clear_artist_sort(artist_1_id.as_ref()).unwrap();
let artist_1: &Artist = MH::get_artist(&music_hoard.collection, &artist_1_id).unwrap();
let artist_2: &Artist = MH::get_artist(&music_hoard.collection, &artist_2_id).unwrap();
assert!(artist_2 < artist_1);
assert_eq!(artist_1, &music_hoard.collection[1]);
assert_eq!(artist_2, &music_hoard.collection[0]);
}
#[test] #[test]
fn collection_error() { fn collection_error() {
@ -584,13 +328,14 @@ mod tests {
#[test] #[test]
fn set_clear_artist_mb_ref() { fn set_clear_artist_mb_ref() {
let mut database = MockIDatabase::new(); let mut database = MockIDatabase::new();
database.expect_save().times(3).returning(|_| Ok(())); database.expect_save().times(2).returning(|_| Ok(()));
let mut artist_id = ArtistId::new("an artist"); let mut artist_id = ArtistId::new("an artist");
let artist_id_2 = ArtistId::new("another artist"); let artist_id_2 = ArtistId::new("another artist");
let mut music_hoard = MusicHoard::database(database); let mut music_hoard = MusicHoard::database(database);
assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); music_hoard.collection.push(Artist::new(artist_id.clone()));
music_hoard.collection.sort_unstable();
let mut expected = ArtistMbRef::None; let mut expected = ArtistMbRef::None;
assert_eq!(music_hoard.collection[0].meta.id.mb_ref, expected); assert_eq!(music_hoard.collection[0].meta.id.mb_ref, expected);
@ -629,13 +374,14 @@ mod tests {
#[test] #[test]
fn set_clear_artist_info() { fn set_clear_artist_info() {
let mut database = MockIDatabase::new(); let mut database = MockIDatabase::new();
database.expect_save().times(3).returning(|_| Ok(())); database.expect_save().times(2).returning(|_| Ok(()));
let artist_id = ArtistId::new("an artist"); let artist_id = ArtistId::new("an artist");
let artist_id_2 = ArtistId::new("another artist"); let artist_id_2 = ArtistId::new("another artist");
let mut music_hoard = MusicHoard::database(database); let mut music_hoard = MusicHoard::database(database);
assert!(music_hoard.add_artist(artist_id.clone()).is_ok()); music_hoard.collection.push(Artist::new(artist_id.clone()));
music_hoard.collection.sort_unstable();
let mut expected = ArtistInfo::default(); let mut expected = ArtistInfo::default();
assert_eq!(music_hoard.collection[0].meta.info, expected); assert_eq!(music_hoard.collection[0].meta.info, expected);
@ -669,97 +415,6 @@ mod tests {
assert_eq!(music_hoard.collection[0].meta.info, expected); assert_eq!(music_hoard.collection[0].meta.info, expected);
} }
#[test]
fn add_to_remove_from_property() {
let mut database = MockIDatabase::new();
database.expect_save().times(3).returning(|_| Ok(()));
let artist_id = ArtistId::new("an artist");
let artist_id_2 = ArtistId::new("another artist");
let mut music_hoard = MusicHoard::database(database);
assert!(music_hoard.add_artist(artist_id.clone()).is_ok());
let mut expected: Vec<String> = vec![];
assert!(music_hoard.collection[0].meta.info.properties.is_empty());
// Adding URLs to an artist not in the collection is an error.
assert!(music_hoard
.add_to_artist_property(&artist_id_2, "MusicButler", vec![MUSICBUTLER])
.is_err());
assert!(music_hoard.collection[0].meta.info.properties.is_empty());
// Adding mutliple URLs without clashes.
assert!(music_hoard
.add_to_artist_property(&artist_id, "MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2])
.is_ok());
expected.push(MUSICBUTLER.to_owned());
expected.push(MUSICBUTLER_2.to_owned());
let info = &music_hoard.collection[0].meta.info;
assert_eq!(info.properties.get("MusicButler"), Some(&expected));
// Removing URLs from an artist not in the collection is an error.
assert!(music_hoard
.remove_from_artist_property(&artist_id_2, "MusicButler", vec![MUSICBUTLER])
.is_err());
let info = &music_hoard.collection[0].meta.info;
assert_eq!(info.properties.get("MusicButler"), Some(&expected));
// Removing multiple URLs without clashes.
assert!(music_hoard
.remove_from_artist_property(
&artist_id,
"MusicButler",
vec![MUSICBUTLER, MUSICBUTLER_2]
)
.is_ok());
expected.clear();
assert!(music_hoard.collection[0].meta.info.properties.is_empty());
}
#[test]
fn set_clear_property() {
let mut database = MockIDatabase::new();
database.expect_save().times(3).returning(|_| Ok(()));
let artist_id = ArtistId::new("an artist");
let artist_id_2 = ArtistId::new("another artist");
let mut music_hoard = MusicHoard::database(database);
assert!(music_hoard.add_artist(artist_id.clone()).is_ok());
let mut expected: Vec<String> = vec![];
assert!(music_hoard.collection[0].meta.info.properties.is_empty());
// Seting URL on an artist not in the collection is an error.
assert!(music_hoard
.set_artist_property(&artist_id_2, "MusicButler", vec![MUSICBUTLER])
.is_err());
assert!(music_hoard.collection[0].meta.info.properties.is_empty());
// Set URLs.
assert!(music_hoard
.set_artist_property(&artist_id, "MusicButler", vec![MUSICBUTLER, MUSICBUTLER_2])
.is_ok());
expected.clear();
expected.push(MUSICBUTLER.to_owned());
expected.push(MUSICBUTLER_2.to_owned());
let info = &music_hoard.collection[0].meta.info;
assert_eq!(info.properties.get("MusicButler"), Some(&expected));
// Clearing URLs on an artist that does not exist is an error.
assert!(music_hoard
.clear_artist_property(&artist_id_2, "MusicButler")
.is_err());
// Clear URLs.
assert!(music_hoard
.clear_artist_property(&artist_id, "MusicButler")
.is_ok());
expected.clear();
assert!(music_hoard.collection[0].meta.info.properties.is_empty());
}
#[test] #[test]
fn album_new_delete() { fn album_new_delete() {
let album_id = AlbumId::new("an album"); let album_id = AlbumId::new("an album");
@ -876,47 +531,6 @@ mod tests {
assert_eq!(album.meta.id.mb_ref, AlbumMbRef::None); assert_eq!(album.meta.id.mb_ref, AlbumMbRef::None);
} }
#[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::new(album_id.clone()));
database
.expect_load()
.times(1)
.return_once(|| Ok(database_result));
database.expect_save().times(2).returning(|_| Ok(()));
let mut music_hoard = MusicHoard::database(database);
music_hoard.reload_database().unwrap();
assert_eq!(music_hoard.collection[0].albums[0].meta.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].meta.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].meta.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].meta.seq, AlbumSeq(0));
}
#[test] #[test]
fn set_clear_album_info() { fn set_clear_album_info() {
let mut database = MockIDatabase::new(); let mut database = MockIDatabase::new();
@ -1031,8 +645,11 @@ mod tests {
let mut music_hoard = MusicHoard::database(database); let mut music_hoard = MusicHoard::database(database);
music_hoard.reload_database().unwrap(); music_hoard.reload_database().unwrap();
let artist_id = ArtistId::new("an artist");
music_hoard.collection.push(Artist::new(artist_id.clone()));
let actual_err = music_hoard let actual_err = music_hoard
.add_artist(ArtistId::new("an artist")) .add_album(artist_id, AlbumMeta::new("an album"))
.unwrap_err(); .unwrap_err();
let expected_err = Error::DatabaseError( let expected_err = Error::DatabaseError(
database::SaveError::IoError(String::from("I/O error")).to_string(), database::SaveError::IoError(String::from("I/O error")).to_string(),