118---limit-the-information-stored-in-the-database (#126)

Closes #118
Closes #68

Reviewed-on: #126

src/core/collection/album.rs

@@ -1,21 +1,19 @@
 use std::mem;
 
-use serde::{Deserialize, Serialize};
-
 use crate::core::collection::{
     merge::{Merge, MergeSorted},
     track::Track,
 };
 
 /// An album is a collection of tracks that were released together.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Album {
     pub id: AlbumId,
     pub tracks: Vec<Track>,
 }
 
 /// The album identifier.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, PartialOrd, Ord, Eq, Hash)]
+#[derive(Clone, Debug, PartialEq, PartialOrd, Ord, Eq, Hash)]
 pub struct AlbumId {
     pub year: u32,
     pub title: String,

src/core/collection/artist.rs

@@ -4,7 +4,6 @@ use std::{
     mem,
 };
 
-use serde::{Deserialize, Serialize};
 use url::Url;
 use uuid::Uuid;
 
@@ -15,7 +14,7 @@ use crate::core::collection::{
 };
 
 /// An artist.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Artist {
     pub id: ArtistId,
     pub sort: Option<ArtistId>,
@@ -25,7 +24,7 @@ pub struct Artist {
 }
 
 /// The artist identifier.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct ArtistId {
     pub name: String,
 }
@@ -185,7 +184,7 @@ pub trait IMbid {
 }
 
 /// MusicBrainz reference.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub struct MusicBrainz(Url);
 
 impl MusicBrainz {

src/core/collection/track.rs

@@ -1,9 +1,7 @@
-use serde::{Deserialize, Serialize};
-
 use crate::core::collection::merge::Merge;
 
 /// A single track on an album.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq)]
 pub struct Track {
     pub id: TrackId,
     pub artist: Vec<String>,
@@ -11,14 +9,14 @@ pub struct Track {
 }
 
 /// The track identifier.
-#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct TrackId {
     pub number: u32,
     pub title: String,
 }
 
 /// The track quality. Combines format and bitrate information.
-#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub struct Quality {
     pub format: Format,
     pub bitrate: u32,
@@ -31,7 +29,7 @@ impl Track {
 }
 
 /// The track file format.
-#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq, Hash)]
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub enum Format {
     Flac,
     Mp3,

src/core/database/json/mod.rs

@@ -10,6 +10,8 @@ use crate::core::{
     database::{IDatabase, LoadError, SaveError},
 };
 
+use super::serde::{deserialize::DeserializeDatabase, serialize::SerializeDatabase};
+
 impl From<serde_json::Error> for LoadError {
     fn from(err: serde_json::Error) -> LoadError {
         LoadError::SerDeError(err.to_string())
@@ -48,11 +50,13 @@ impl<JDB: IJsonDatabaseBackend> JsonDatabase<JDB> {
 impl<JDB: IJsonDatabaseBackend> IDatabase for JsonDatabase<JDB> {
     fn load(&self) -> Result<Collection, LoadError> {
         let serialized = self.backend.read()?;
-        Ok(serde_json::from_str(&serialized)?)
+        let database: DeserializeDatabase = serde_json::from_str(&serialized)?;
+        database.try_into()
     }
 
     fn save(&mut self, collection: &Collection) -> Result<(), SaveError> {
-        let serialized = serde_json::to_string(&collection)?;
+        let database: SerializeDatabase = collection.into();
+        let serialized = serde_json::to_string(&database)?;
         self.backend.write(&serialized)?;
         Ok(())
     }
@@ -65,20 +69,42 @@ pub mod testmod;
 mod tests {
     use std::collections::HashMap;
 
+    use mockall::predicate;
+
     use crate::core::{
-        collection::{
+        collection::{artist::Artist, Collection},
-            artist::{Artist, ArtistId},
-            Collection,
-        },
         testmod::FULL_COLLECTION,
     };
 
     use super::*;
     use testmod::DATABASE_JSON;
 
+    fn expected() -> Collection {
+        let mut expected = FULL_COLLECTION.to_owned();
+        for artist in expected.iter_mut() {
+            artist.albums.clear();
+        }
+        expected
+    }
+
+    #[test]
+    fn save() {
+        let write_data = FULL_COLLECTION.to_owned();
+        let input = DATABASE_JSON.to_owned();
+
+        let mut backend = MockIJsonDatabaseBackend::new();
+        backend
+            .expect_write()
+            .with(predicate::eq(input))
+            .times(1)
+            .return_once(|_| Ok(()));
+
+        JsonDatabase::new(backend).save(&write_data).unwrap();
+    }
+
     #[test]
     fn load() {
-        let expected = FULL_COLLECTION.to_owned();
+        let expected = expected();
         let result = Ok(DATABASE_JSON.to_owned());
 
         let mut backend = MockIJsonDatabaseBackend::new();
@@ -91,37 +117,31 @@ mod tests {
 
     #[test]
     fn reverse() {
-        // Saving is non-deterministic due to HashMap, but regardless of how the data ends up being
+        let input = DATABASE_JSON.to_owned();
-        // saved, loading it again should always yield the exact same data as was input.
+        let result = Ok(input.clone());
-        struct MockIJsonDatabaseBackend {
-            data: Option<String>,
-        }
 
-        impl IJsonDatabaseBackend for MockIJsonDatabaseBackend {
+        let mut backend = MockIJsonDatabaseBackend::new();
-            fn write(&mut self, json: &str) -> Result<(), std::io::Error> {
+        backend
-                let _ = self.data.insert(json.to_owned());
+            .expect_write()
-                Ok(())
+            .with(predicate::eq(input))
-            }
+            .times(1)
-
+            .return_once(|_| Ok(()));
-            fn read(&self) -> Result<String, std::io::Error> {
+        backend.expect_read().times(1).return_once(|| result);
-                Ok(self.data.as_ref().unwrap().clone())
-            }
-        }
-
-        let backend = MockIJsonDatabaseBackend { data: None };
         let mut database = JsonDatabase::new(backend);
 
         let write_data = FULL_COLLECTION.to_owned();
         database.save(&write_data).unwrap();
         let read_data: Vec<Artist> = database.load().unwrap();
 
-        assert_eq!(write_data, read_data);
+        // Album information is not saved to disk.
+        let expected = expected();
+        assert_eq!(read_data, expected);
     }
 
     #[test]
     fn load_errors() {
         let json = String::from("");
-        let serde_err = serde_json::from_str::<Collection>(&json);
+        let serde_err = serde_json::from_str::<DeserializeDatabase>(&json);
         assert!(serde_err.is_err());
 
         let serde_err: LoadError = serde_err.unwrap_err().into();
@@ -131,11 +151,9 @@ mod tests {
 
     #[test]
     fn save_errors() {
-        let mut object = HashMap::<ArtistId, String>::new();
+        // serde_json will raise an error as it requires keys to be strings.
-        object.insert(
+        let mut object = HashMap::<Option<String>, String>::new();
-            ArtistId::new(String::from("artist")),
+        object.insert(Some(String::from("artist")), String::from("string"));
-            String::from("string"),
-        );
         let serde_err = serde_json::to_string(&object);
         assert!(serde_err.is_err());
 

src/core/database/json/testmod.rs

@@ -1,57 +1,17 @@
-pub static DATABASE_JSON: &str = "[\
+pub static DATABASE_JSON: &str = "{\
+    \"V20240210\":\
+    [\
     {\
-    \"id\":{\"name\":\"album_artist a\"},\
+    \"name\":\"album_artist a\",\
     \"sort\":null,\
     \"musicbrainz\":\"https://musicbrainz.org/artist/00000000-0000-0000-0000-000000000000\",\
     \"properties\":{\
     \"MusicButler\":[\"https://www.musicbutler.io/artist-page/000000000\"],\
     \"Qobuz\":[\"https://www.qobuz.com/nl-nl/interpreter/artist-a/download-streaming-albums\"]\
-    },\
-    \"albums\":[\
-    {\
-    \"id\":{\"year\":1998,\"title\":\"album_title a.a\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track a.a.1\"},\
-    \"artist\":[\"artist a.a.1\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":992}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track a.a.2\"},\
-    \"artist\":[\"artist a.a.2.1\",\"artist a.a.2.2\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":320}\
-    },\
-    {\
-    \"id\":{\"number\":3,\"title\":\"track a.a.3\"},\
-    \"artist\":[\"artist a.a.3\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":1061}\
-    },\
-    {\
-    \"id\":{\"number\":4,\"title\":\"track a.a.4\"},\
-    \"artist\":[\"artist a.a.4\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":1042}\
     }\
-    ]\
     },\
     {\
-    \"id\":{\"year\":2015,\"title\":\"album_title a.b\"},\
+    \"name\":\"album_artist b\",\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track a.b.1\"},\
-    \"artist\":[\"artist a.b.1\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":1004}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track a.b.2\"},\
-    \"artist\":[\"artist a.b.2\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":1077}\
-    }\
-    ]\
-    }\
-    ]\
-    },\
-    {\
-    \"id\":{\"name\":\"album_artist b\"},\
     \"sort\":null,\
     \"musicbrainz\":\"https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111\",\
     \"properties\":{\
@@ -61,144 +21,19 @@ pub static DATABASE_JSON: &str = "[\
     \"https://www.musicbutler.io/artist-page/111111112\"\
     ],\
     \"Qobuz\":[\"https://www.qobuz.com/nl-nl/interpreter/artist-b/download-streaming-albums\"]\
-    },\
-    \"albums\":[\
-    {\
-    \"id\":{\"year\":2003,\"title\":\"album_title b.a\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track b.a.1\"},\
-    \"artist\":[\"artist b.a.1\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":190}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track b.a.2\"},\
-    \"artist\":[\"artist b.a.2.1\",\"artist b.a.2.2\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":120}\
     }\
-    ]\
     },\
     {\
-    \"id\":{\"year\":2008,\"title\":\"album_title b.b\"},\
+    \"name\":\"album_artist c\",\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track b.b.1\"},\
-    \"artist\":[\"artist b.b.1\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":1077}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track b.b.2\"},\
-    \"artist\":[\"artist b.b.2.1\",\"artist b.b.2.2\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":320}\
-    }\
-    ]\
-    },\
-    {\
-    \"id\":{\"year\":2009,\"title\":\"album_title b.c\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track b.c.1\"},\
-    \"artist\":[\"artist b.c.1\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":190}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track b.c.2\"},\
-    \"artist\":[\"artist b.c.2.1\",\"artist b.c.2.2\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":120}\
-    }\
-    ]\
-    },\
-    {\
-    \"id\":{\"year\":2015,\"title\":\"album_title b.d\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track b.d.1\"},\
-    \"artist\":[\"artist b.d.1\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":190}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track b.d.2\"},\
-    \"artist\":[\"artist b.d.2.1\",\"artist b.d.2.2\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":120}\
-    }\
-    ]\
-    }\
-    ]\
-    },\
-    {\
-    \"id\":{\"name\":\"album_artist c\"},\
     \"sort\":null,\
     \"musicbrainz\":\"https://musicbrainz.org/artist/11111111-1111-1111-1111-111111111111\",\
-    \"properties\":{},\
+    \"properties\":{}\
-    \"albums\":[\
-    {\
-    \"id\":{\"year\":1985,\"title\":\"album_title c.a\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track c.a.1\"},\
-    \"artist\":[\"artist c.a.1\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":320}\
     },\
     {\
-    \"id\":{\"number\":2,\"title\":\"track c.a.2\"},\
+    \"name\":\"album_artist d\",\
-    \"artist\":[\"artist c.a.2.1\",\"artist c.a.2.2\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":120}\
-    }\
-    ]\
-    },\
-    {\
-    \"id\":{\"year\":2018,\"title\":\"album_title c.b\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track c.b.1\"},\
-    \"artist\":[\"artist c.b.1\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":1041}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track c.b.2\"},\
-    \"artist\":[\"artist c.b.2.1\",\"artist c.b.2.2\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":756}\
-    }\
-    ]\
-    }\
-    ]\
-    },\
-    {\
-    \"id\":{\"name\":\"album_artist d\"},\
     \"sort\":null,\
     \"musicbrainz\":null,\
-    \"properties\":{},\
+    \"properties\":{}\
-    \"albums\":[\
-    {\
-    \"id\":{\"year\":1995,\"title\":\"album_title d.a\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track d.a.1\"},\
-    \"artist\":[\"artist d.a.1\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":120}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track d.a.2\"},\
-    \"artist\":[\"artist d.a.2.1\",\"artist d.a.2.2\"],\
-    \"quality\":{\"format\":\"Mp3\",\"bitrate\":120}\
     }\
     ]\
-    },\
+    }";
-    {\
-    \"id\":{\"year\":2028,\"title\":\"album_title d.b\"},\
-    \"tracks\":[\
-    {\
-    \"id\":{\"number\":1,\"title\":\"track d.b.1\"},\
-    \"artist\":[\"artist d.b.1\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":841}\
-    },\
-    {\
-    \"id\":{\"number\":2,\"title\":\"track d.b.2\"},\
-    \"artist\":[\"artist d.b.2.1\",\"artist d.b.2.2\"],\
-    \"quality\":{\"format\":\"Flac\",\"bitrate\":756}\
-    }\
-    ]\
-    }\
-    ]\
-    }\
-    ]";

src/core/database/mod.rs

@@ -2,13 +2,15 @@
 
 #[cfg(feature = "database-json")]
 pub mod json;
+#[cfg(feature = "database-json")]
+mod serde;
 
 use std::fmt;
 
 #[cfg(test)]
 use mockall::automock;
 
-use crate::core::collection::Collection;
+use crate::core::collection::{self, Collection};
 
 /// Trait for interacting with the database.
 #[cfg_attr(test, automock)]
@@ -59,6 +61,14 @@ impl From<std::io::Error> for LoadError {
     }
 }
 
+impl From<collection::Error> for LoadError {
+    fn from(err: collection::Error) -> Self {
+        match err {
+            collection::Error::UrlError(e) => LoadError::SerDeError(e),
+        }
+    }
+}
+
 /// Error type for database calls.
 #[derive(Debug)]
 pub enum SaveError {
@@ -109,6 +119,10 @@ mod tests {
         assert!(!io_err.to_string().is_empty());
         assert!(!format!("{:?}", io_err).is_empty());
 
+        let col_err: LoadError = collection::Error::UrlError(String::from("get rekt")).into();
+        assert!(!col_err.to_string().is_empty());
+        assert!(!format!("{:?}", col_err).is_empty());
+
         let io_err: SaveError = io::Error::new(io::ErrorKind::Interrupted, "error").into();
         assert!(!io_err.to_string().is_empty());
         assert!(!format!("{:?}", io_err).is_empty());

src/core/database/serde/deserialize.rs

@@ -0,0 +1,48 @@
+use std::collections::HashMap;
+
+use serde::Deserialize;
+
+use crate::{
+    collection::artist::{ArtistId, MusicBrainz},
+    core::{
+        collection::{artist::Artist, Collection},
+        database::{serde::Database, LoadError},
+    },
+};
+
+pub type DeserializeDatabase = Database<DeserializeArtist>;
+
+#[derive(Debug, Deserialize)]
+pub struct DeserializeArtist {
+    name: String,
+    sort: Option<String>,
+    musicbrainz: Option<String>,
+    properties: HashMap<String, Vec<String>>,
+}
+
+impl TryFrom<DeserializeDatabase> for Collection {
+    type Error = LoadError;
+
+    fn try_from(database: DeserializeDatabase) -> Result<Self, Self::Error> {
+        match database {
+            Database::V20240210(collection) => collection
+                .into_iter()
+                .map(|artist| artist.try_into())
+                .collect(),
+        }
+    }
+}
+
+impl TryFrom<DeserializeArtist> for Artist {
+    type Error = LoadError;
+
+    fn try_from(artist: DeserializeArtist) -> Result<Self, Self::Error> {
+        Ok(Artist {
+            id: ArtistId::new(artist.name),
+            sort: artist.sort.map(ArtistId::new),
+            musicbrainz: artist.musicbrainz.map(MusicBrainz::new).transpose()?,
+            properties: artist.properties,
+            albums: vec![],
+        })
+    }
+}

src/core/database/serde/mod.rs

@@ -0,0 +1,11 @@
+//! Helper module for backends that can use serde for (de)serialisation.
+
+pub mod deserialize;
+pub mod serialize;
+
+use serde::{Deserialize, Serialize};
+
+#[derive(Debug, Serialize, Deserialize)]
+pub enum Database<ARTIST> {
+    V20240210(Vec<ARTIST>),
+}

src/core/database/serde/serialize.rs

@@ -0,0 +1,39 @@
+use std::collections::BTreeMap;
+
+use serde::Serialize;
+
+use crate::core::{
+    collection::{artist::Artist, Collection},
+    database::serde::Database,
+};
+
+pub type SerializeDatabase<'a> = Database<SerializeArtist<'a>>;
+
+#[derive(Debug, Serialize)]
+pub struct SerializeArtist<'a> {
+    name: &'a str,
+    sort: Option<&'a str>,
+    musicbrainz: Option<&'a str>,
+    properties: BTreeMap<&'a str, &'a Vec<String>>,
+}
+
+impl<'a> From<&'a Collection> for SerializeDatabase<'a> {
+    fn from(collection: &'a Collection) -> Self {
+        Database::V20240210(collection.iter().map(|artist| artist.into()).collect())
+    }
+}
+
+impl<'a> From<&'a Artist> for SerializeArtist<'a> {
+    fn from(artist: &'a Artist) -> Self {
+        SerializeArtist {
+            name: &artist.id.name,
+            sort: artist.sort.as_ref().map(|id| id.name.as_ref()),
+            musicbrainz: artist.musicbrainz.as_ref().map(|mb| mb.as_ref()),
+            properties: artist
+                .properties
+                .iter()
+                .map(|(k, v)| (k.as_ref(), v))
+                .collect(),
+        }
+    }
+}

tests/database/json.rs

@@ -4,7 +4,7 @@ use once_cell::sync::Lazy;
 use tempfile::NamedTempFile;
 
 use musichoard::{
-    collection::artist::Artist,
+    collection::{artist::Artist, Collection},
     database::{
         json::{backend::JsonDatabaseFileBackend, JsonDatabase},
         IDatabase,
@@ -16,6 +16,30 @@ use crate::testlib::COLLECTION;
 pub static DATABASE_TEST_FILE: Lazy<PathBuf> =
     Lazy::new(|| fs::canonicalize("./tests/files/database/database.json").unwrap());
 
+fn expected() -> Collection {
+    let mut expected = COLLECTION.to_owned();
+    for artist in expected.iter_mut() {
+        artist.albums.clear();
+    }
+    expected
+}
+
+#[test]
+fn save() {
+    let file = NamedTempFile::new().unwrap();
+
+    let backend = JsonDatabaseFileBackend::new(file.path());
+    let mut database = JsonDatabase::new(backend);
+
+    let write_data = COLLECTION.to_owned();
+    database.save(&write_data).unwrap();
+
+    let expected = fs::read_to_string(&*DATABASE_TEST_FILE).unwrap();
+    let actual = fs::read_to_string(file.path()).unwrap();
+
+    assert_eq!(actual, expected);
+}
+
 #[test]
 fn load() {
     let backend = JsonDatabaseFileBackend::new(&*DATABASE_TEST_FILE);
@@ -23,14 +47,12 @@ fn load() {
 
     let read_data: Vec<Artist> = database.load().unwrap();
 
-    let expected = COLLECTION.to_owned();
+    let expected = expected();
     assert_eq!(read_data, expected);
 }
 
 #[test]
 fn reverse() {
-    // Saving is non-deterministic due to HashMap, but regardless of how the data ends up being
-    // saved, loading it again should always yield the exact same data as was input.
     let file = NamedTempFile::new().unwrap();
 
     let backend = JsonDatabaseFileBackend::new(file.path());
@@ -40,5 +62,7 @@ fn reverse() {
     database.save(&write_data).unwrap();
     let read_data: Vec<Artist> = database.load().unwrap();
 
-    assert_eq!(write_data, read_data);
+    // Album data is not saved into database.
+    let expected = expected();
+    assert_eq!(read_data, expected);
 }