From cfa0d8f256b0bbf68be6cf3f8a741e76707105bc Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Fri, 10 Jan 2025 19:20:29 +0100 Subject: [PATCH 01/20] Update build and dev dependencies --- Cargo.lock | 9 ++++----- Cargo.toml | 7 +++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8abaead..9fc3e96 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -868,14 +868,13 @@ dependencies = [ [[package]] name = "mockall" -version = "0.12.1" +version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43766c2b5203b10de348ffe19f7e54564b64f3d6018ff7648d1e2d6d3a0f0a48" +checksum = "39a6bfcc6c8c7eed5ee98b9c3e33adc726054389233e201c95dab2d41a3839d2" dependencies = [ "cfg-if", "downcast", "fragile", - "lazy_static", "mockall_derive", "predicates", "predicates-tree", @@ -883,9 +882,9 @@ dependencies = [ [[package]] name = "mockall_derive" -version = "0.12.1" +version = "0.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af7cbce79ec385a1d4f54baa90a76401eb15d9cab93685f62e7e9f942aa00ae2" +checksum = "25ca3004c2efe9011bd4e461bd8256445052b9615405b4f7ea43fc8ca5c20898" dependencies = [ "cfg-if", "proc-macro2", diff --git a/Cargo.toml b/Cargo.toml index cbc46c2..0fc844c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,12 +23,11 @@ url = { version = "2.5.4" } uuid = { version = "1.11.0" } [build-dependencies] -version_check = "0.9.4" +version_check = "0.9.5" [dev-dependencies] -mockall = "0.12.1" -once_cell = "1.19.0" -tempfile = "3.10.0" +mockall = "0.13.1" +tempfile = "3.15.0" [features] default = ["database-json", "library-beets"] -- 2.47.1 From 3c6c093f1e8723e3ff81513d93961f605a31d233 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Fri, 10 Jan 2025 19:25:40 +0100 Subject: [PATCH 02/20] Add rusqlite dependency --- Cargo.lock | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- Cargo.toml | 2 ++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9fc3e96..a260e3e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -17,6 +17,18 @@ version = "2.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "512761e0bb2578dd7380c6baaa0f4ce03e84f95e960231d1dec8bf4d7d6e2627" +[[package]] +name = "ahash" +version = "0.8.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e89da841a80418a9b391ebaea17f5c112ffaaa96f621d2c285b5174da76b9011" +dependencies = [ + "cfg-if", + "once_cell", + "version_check", + "zerocopy", +] + [[package]] name = "aho-corasick" version = "1.1.3" @@ -298,6 +310,18 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "fallible-iterator" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" + +[[package]] +name = "fallible-streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" + [[package]] name = "fastrand" version = "2.3.0" @@ -432,6 +456,15 @@ dependencies = [ "tracing", ] +[[package]] +name = "hashbrown" +version = "0.14.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5274423e17b7c9fc20b6e7e208532f9b19825d82dfd615708b70edd83df41f1" +dependencies = [ + "ahash", +] + [[package]] name = "hashbrown" version = "0.15.2" @@ -443,6 +476,15 @@ dependencies = [ "foldhash", ] +[[package]] +name = "hashlink" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ba4ff7128dee98c7dc9794b6a411377e1404dba1c97deb8d1a55297bd25d8af" +dependencies = [ + "hashbrown 0.14.5", +] + [[package]] name = "heck" version = "0.3.3" @@ -731,7 +773,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "62f822373a4fe84d4bb149bf54e584a7f4abec90e072ed49cda0edea5b95471f" dependencies = [ "equivalent", - "hashbrown", + "hashbrown 0.15.2", ] [[package]] @@ -796,6 +838,16 @@ version = "0.2.169" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b5aba8db14291edd000dfcc4d620c7ebfb122c613afb886ca8803fa4e128a20a" +[[package]] +name = "libsqlite3-sys" +version = "0.30.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e99fb7a497b1e3339bc746195567ed8d3e24945ecd636e3619d20b9de9e9149" +dependencies = [ + "pkg-config", + "vcpkg", +] + [[package]] name = "linux-raw-sys" version = "0.4.14" @@ -830,7 +882,7 @@ version = "0.12.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "234cf4f4a04dc1f57e24b96cc0cd600cf2af460d4161ac5ecdd0af8e1f3b2a38" dependencies = [ - "hashbrown", + "hashbrown 0.15.2", ] [[package]] @@ -904,6 +956,7 @@ dependencies = [ "paste", "ratatui", "reqwest", + "rusqlite", "serde", "serde_json", "structopt", @@ -1254,6 +1307,20 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rusqlite" +version = "0.32.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7753b721174eb8ff87a9a0e799e2d7bc3749323e773db92e0984debb00019d6e" +dependencies = [ + "bitflags 2.6.0", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "smallvec", +] + [[package]] name = "rustc-demangle" version = "0.1.24" @@ -2208,6 +2275,26 @@ dependencies = [ "synstructure", ] +[[package]] +name = "zerocopy" +version = "0.7.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1b9b4fd18abc82b8136838da5d50bae7bdea537c574d8dc1a34ed098d6c166f0" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.7.35" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa4f8080344d4671fb4e831a13ad1e68092748387dfc4f55e356242fae12ce3e" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.95", +] + [[package]] name = "zerofrom" version = "0.1.5" diff --git a/Cargo.toml b/Cargo.toml index 0fc844c..5d159f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,7 @@ openssh = { version = "0.11.4", features = ["native-mux"], default-features = fa paste = { version = "1.0.15", optional = true } ratatui = { version = "0.29.0", optional = true} reqwest = { version = "0.12.12", features = ["blocking", "json"], optional = true } +rusqlite = { version = "0.32.1", optional = true } serde = { version = "1.0.217", features = ["derive"], optional = true } serde_json = { version = "1.0.134", optional = true} structopt = { version = "0.3.26", optional = true} @@ -32,6 +33,7 @@ tempfile = "3.15.0" [features] default = ["database-json", "library-beets"] bin = ["structopt"] +database-sqlite = ["rusqlite"] database-json = ["serde", "serde_json"] library-beets = [] library-beets-ssh = ["openssh", "tokio"] -- 2.47.1 From 02fdef444f912d9626205c93e34a4001a01933e0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 11:36:54 +0100 Subject: [PATCH 03/20] Draft code to write database --- src/external/database/mod.rs | 3 + src/external/database/serde/common.rs | 2 +- src/external/database/serde/serialize.rs | 24 +-- src/external/database/sql/backend.rs | 166 ++++++++++++++ src/external/database/sql/mod.rs | 263 +++++++++++++++++++++++ 5 files changed, 445 insertions(+), 13 deletions(-) create mode 100644 src/external/database/sql/backend.rs create mode 100644 src/external/database/sql/mod.rs diff --git a/src/external/database/mod.rs b/src/external/database/mod.rs index 8bc349c..72a8931 100644 --- a/src/external/database/mod.rs +++ b/src/external/database/mod.rs @@ -1,4 +1,7 @@ #[cfg(feature = "database-json")] pub mod json; +#[cfg(feature = "database-sqlite")] +pub mod sql; + #[cfg(feature = "database-json")] mod serde; diff --git a/src/external/database/serde/common.rs b/src/external/database/serde/common.rs index 1cc230e..e46b3de 100644 --- a/src/external/database/serde/common.rs +++ b/src/external/database/serde/common.rs @@ -37,7 +37,7 @@ pub struct AlbumDateDef { } #[derive(Debug, Deserialize, Serialize)] -pub struct SerdeAlbumDate(#[serde(with = "AlbumDateDef")] AlbumDate); +pub struct SerdeAlbumDate(#[serde(with = "AlbumDateDef")] pub AlbumDate); impl From for AlbumDate { fn from(value: SerdeAlbumDate) -> Self { diff --git a/src/external/database/serde/serialize.rs b/src/external/database/serde/serialize.rs index a88657f..aa9b336 100644 --- a/src/external/database/serde/serialize.rs +++ b/src/external/database/serde/serialize.rs @@ -24,22 +24,22 @@ impl<'a> From<&'a Collection> for SerializeDatabase<'a> { #[derive(Debug, Serialize)] pub struct SerializeArtist<'a> { - name: &'a str, - sort: Option<&'a str>, - musicbrainz: SerializeMbRefOption<'a>, - properties: BTreeMap<&'a str, &'a Vec>, - albums: Vec>, + pub name: &'a str, + pub sort: Option<&'a str>, + pub musicbrainz: SerializeMbRefOption<'a>, + pub properties: BTreeMap<&'a str, &'a Vec>, + pub albums: Vec>, } #[derive(Debug, Serialize)] pub struct SerializeAlbum<'a> { - title: &'a str, - lib_id: SerdeAlbumLibId, - date: SerdeAlbumDate, - seq: u8, - musicbrainz: SerializeMbRefOption<'a>, - primary_type: Option, - secondary_types: Vec, + pub title: &'a str, + pub lib_id: SerdeAlbumLibId, + pub date: SerdeAlbumDate, + pub seq: u8, + pub musicbrainz: SerializeMbRefOption<'a>, + pub primary_type: Option, + pub secondary_types: Vec, } #[derive(Debug, Serialize)] diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs new file mode 100644 index 0000000..763d267 --- /dev/null +++ b/src/external/database/sql/backend.rs @@ -0,0 +1,166 @@ +//! Module for storing MusicHoard data in a JSON file database. + +use std::path::PathBuf; + +use rusqlite::{self, CachedStatement, Connection, Params, Statement}; + +use crate::external::database::{ + serde::serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, + sql::{Error, ISqlDatabaseBackend}, +}; + +/// SQLite database backend that uses SQLite as the implementation. +pub struct SqlDatabaseSqliteBackend { + conn: Connection, +} + +impl SqlDatabaseSqliteBackend { + /// Create a [`SqlDatabaseSqliteBackend`] that will read/write to the provided database. + pub fn new>(path: P) -> Result { + Ok(SqlDatabaseSqliteBackend { + conn: Connection::open(path.into()).map_err(|err| Error::OpenError(err.to_string()))?, + }) + } + + fn prepare(&self, sql: &str) -> Result { + self.conn + .prepare(sql) + .map_err(|err| Error::StmtError(err.to_string())) + } + + fn prepare_cached(&self, sql: &str) -> Result { + self.conn + .prepare_cached(sql) + .map_err(|err| Error::StmtError(err.to_string())) + } + + fn execute(stmt: &mut Statement, params: P) -> Result<(), Error> { + stmt.execute(params) + .map(|_| ()) + .map_err(|err| Error::ExecError(err.to_string())) + } +} + +impl From for Error { + fn from(value: serde_json::Error) -> Self { + Error::SerDeError(value.to_string()) + } +} + +impl ISqlDatabaseBackend for SqlDatabaseSqliteBackend { + fn begin_transaction(&self) -> Result<(), Error> { + let mut stmt = self.prepare_cached("BEGIN TRANSACTION;")?; + Self::execute(&mut stmt, ()) + } + + fn commit_transaction(&self) -> Result<(), Error> { + let mut stmt = self.prepare_cached("COMMIT;")?; + Self::execute(&mut stmt, ()) + } + + fn create_database_metadata_table(&self) -> Result<(), Error> { + let mut stmt = self.prepare( + "CREATE TABLE IF NOT EXISTS database_metadata ( + name TEXT NOT NULL PRIMARY KEY, + value TEXT NOT NULL + )", + )?; + Self::execute(&mut stmt, ()) + } + + fn drop_database_metadata_table(&self) -> Result<(), Error> { + let mut stmt = self.prepare_cached("DROP TABLE database_metadata")?; + Self::execute(&mut stmt, ()) + } + + fn create_artists_table(&self) -> Result<(), Error> { + let mut stmt = self.prepare( + "CREATE TABLE IF NOT EXISTS artists ( + name TEXT NOT NULL PRIMARY KEY, + sort TEXT NULL, + mbid JSON NOT NULL DEFAULT '\"None\"', + properties JSON NOT NULL DEFAULT '{}' + )", + )?; + Self::execute(&mut stmt, ()) + } + + fn drop_artists_table(&self) -> Result<(),Error> { + let mut stmt = self.prepare_cached("DROP TABLE artists")?; + Self::execute(&mut stmt, ()) + } + + fn create_albums_table(&self) -> Result<(), Error> { + let mut stmt = self.prepare( + "CREATE TABLE IF NOT EXISTS albums ( + title TEXT NOT NULL PRIMARY KEY, + lib_id JSON NOT NULL DEFAULT '\"None\"', + mbid JSON NOT NULL DEFAULT '\"None\"', + artist_name TEXT NOT NULL, + year INT NULL, + month INT NULL, + day INT NULL, + seq INT NOT NULL, + primary_type JSON NOT NULL DEFAULT 'null', + secondary_types JSON NOT NULL DEFAULT '[]', + FOREIGN KEY (artist_name) REFERENCES artists(name) + )", + )?; + Self::execute(&mut stmt, ()) + } + + fn drop_albums_table(&self) -> Result<(),Error> { + let mut stmt = self.prepare_cached("DROP TABLE albums")?; + Self::execute(&mut stmt, ()) + } + + fn insert_database_version<'a>(&self, version: &SerializeDatabase<'a>) -> Result<(), Error> { + let mut stmt = self.prepare_cached( + "INSERT INTO database_metadata (name, value) + VALUES (?1, ?2)", + )?; + let version = match version { + SerializeDatabase::V20250103(_) => "V20250103", + }; + Self::execute(&mut stmt, ("version", version)) + } + + fn insert_artist<'a>(&self, artist: &SerializeArtist<'a>) -> Result<(), Error> { + let mut stmt = self.prepare_cached( + "INSERT INTO artists (name, sort, mbid, properties) + VALUES (?1, ?2, ?3, ?4)", + )?; + Self::execute( + &mut stmt, + ( + artist.name, + artist.sort, + serde_json::to_string(&artist.musicbrainz)?, + serde_json::to_string(&artist.properties)?, + ), + ) + } + + fn insert_album<'a>(&self, artist_name: &str, album: &SerializeAlbum<'a>) -> Result<(), Error> { + let mut stmt = self.prepare_cached( + "INSERT INTO albums (title, lib_id, mbid, artist_name, + year, month, day, seq, primary_type, secondary_types) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)", + )?; + Self::execute( + &mut stmt, + ( + album.title, + serde_json::to_string(&album.lib_id)?, + serde_json::to_string(&album.musicbrainz)?, + artist_name, + album.date.0.year, + album.date.0.month, + album.date.0.day, + album.seq, + serde_json::to_string(&album.primary_type)?, + serde_json::to_string(&album.secondary_types)?, + ), + ) + } +} diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs new file mode 100644 index 0000000..932c180 --- /dev/null +++ b/src/external/database/sql/mod.rs @@ -0,0 +1,263 @@ +//! Module for storing MusicHoard data in a SQLdatabase. + +pub mod backend; + +use std::fmt; + +#[cfg(test)] +use mockall::automock; + +use crate::{ + core::{ + collection::Collection, + interface::database::{IDatabase, LoadError, SaveError}, + }, + external::database::serde::serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, +}; + +/// Trait for the SQL database backend. +#[cfg_attr(test, automock)] +pub trait ISqlDatabaseBackend { + /// Begin an SQL transaction. + fn begin_transaction(&self) -> Result<(), Error>; + + /// Commit ongoing transaction. + fn commit_transaction(&self) -> Result<(), Error>; + + /// Create the database metadata table (if needed). + fn create_database_metadata_table(&self) -> Result<(), Error>; + + /// Drop the database metadata table. + fn drop_database_metadata_table(&self) -> Result<(), Error>; + + /// Create the artists table (if needed). + fn create_artists_table(&self) -> Result<(), Error>; + + /// Drop the artists table. + fn drop_artists_table(&self) -> Result<(), Error>; + + /// Create the albums table (if needed). + fn create_albums_table(&self) -> Result<(), Error>; + + /// Drop the albums table. + fn drop_albums_table(&self) -> Result<(), Error>; + + /// Set the database version. + fn insert_database_version<'a>(&self, version: &SerializeDatabase<'a>) -> Result<(), Error>; + + /// Insert an artist into the artist table. + fn insert_artist<'a>(&self, artist: &SerializeArtist<'a>) -> Result<(), Error>; + + /// Insert an artist into the artist table. + fn insert_album<'a>(&self, artist_name: &str, album: &SerializeAlbum<'a>) -> Result<(), Error>; +} + +/// Errors for SQL database backend. +#[derive(Debug)] +pub enum Error { + /// An error occurred when connecting to the database. + OpenError(String), + /// An error occurred when preparing a statement for execution. + StmtError(String), + /// An error occurred during serialisation. + SerDeError(String), + /// An error occurred during execution. + ExecError(String), +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match *self { + Self::OpenError(ref s) => { + write!(f, "an error occurred when connecting to the database: {s}") + } + Self::StmtError(ref s) => write!( + f, + "an error occurred when preparing a statement for execution: {s}" + ), + Self::SerDeError(ref s) => write!(f, "an error occurred during serialisation : {s}"), + Self::ExecError(ref s) => write!(f, "an error occurred during execution: {s}"), + } + } +} + +impl From for SaveError { + fn from(value: Error) -> Self { + match value { + Error::SerDeError(s) => SaveError::SerDeError(s), + _ => SaveError::IoError(value.to_string()), + } + } +} + +/// SQL database. +pub struct SqlDatabase { + backend: SDB, +} + +impl SqlDatabase { + /// Create a new SQL database with the provided backend, e.g. + /// [`backend::SqlDatabaseSqliteBackend`]. + pub fn new(backend: SDB) -> Result { + let db = SqlDatabase { backend }; + db.begin_transaction()?; + db.create_tables()?; + db.commit_transaction()?; + Ok(db) + } + + fn begin_transaction(&self) -> Result<(), Error> { + self.backend.begin_transaction() + } + + fn commit_transaction(&self) -> Result<(), Error> { + self.backend.commit_transaction() + } + + fn create_tables(&self) -> Result<(), Error> { + self.backend.create_database_metadata_table()?; + self.backend.create_artists_table()?; + self.backend.create_albums_table()?; + Ok(()) + } + + fn drop_tables(&self) -> Result<(), Error> { + self.backend.drop_database_metadata_table()?; + self.backend.drop_artists_table()?; + self.backend.drop_albums_table()?; + Ok(()) + } +} + +impl IDatabase for SqlDatabase { + fn load(&self) -> Result { + Ok(vec![]) + } + + fn save(&mut self, collection: &Collection) -> Result<(), SaveError> { + let database: SerializeDatabase = collection.into(); + self.begin_transaction()?; + + self.drop_tables()?; + self.create_tables()?; + + self.backend.insert_database_version(&database)?; + match database { + SerializeDatabase::V20250103(artists) => { + for artist in artists.iter() { + self.backend.insert_artist(artist)?; + for album in artist.albums.iter() { + self.backend.insert_album(artist.name, album)?; + } + } + } + } + + self.commit_transaction()?; + Ok(()) + } +} + +// #[cfg(test)] +// pub mod testmod; + +// #[cfg(test)] +// mod tests { +// use std::collections::HashMap; + +// use mockall::predicate; + +// use crate::core::{ +// collection::{artist::Artist, 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() { +// for album in artist.albums.iter_mut() { +// album.tracks.clear(); +// } +// } +// expected +// } + +// #[test] +// fn save() { +// let write_data = FULL_COLLECTION.to_owned(); +// let input = DATABASE_JSON.to_owned(); + +// let mut backend = MockISqlDatabaseBackend::new(); +// backend +// .expect_write() +// .with(predicate::eq(input)) +// .times(1) +// .return_once(|_| Ok(())); + +// SqlDatabase::new(backend).save(&write_data).unwrap(); +// } + +// #[test] +// fn load() { +// let expected = expected(); +// let result = Ok(DATABASE_JSON.to_owned()); +// eprintln!("{DATABASE_JSON}"); + +// let mut backend = MockISqlDatabaseBackend::new(); +// backend.expect_read().times(1).return_once(|| result); + +// let read_data: Vec = SqlDatabase::new(backend).load().unwrap(); + +// assert_eq!(read_data, expected); +// } + +// #[test] +// fn reverse() { +// let input = DATABASE_JSON.to_owned(); +// let result = Ok(input.clone()); + +// let mut backend = MockISqlDatabaseBackend::new(); +// backend +// .expect_write() +// .with(predicate::eq(input)) +// .times(1) +// .return_once(|_| Ok(())); +// backend.expect_read().times(1).return_once(|| result); +// let mut database = SqlDatabase::new(backend); + +// let write_data = FULL_COLLECTION.to_owned(); +// database.save(&write_data).unwrap(); +// let read_data: Vec = database.load().unwrap(); + +// // 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::(&json); +// assert!(serde_err.is_err()); + +// let serde_err: LoadError = serde_err.unwrap_err().into(); +// assert!(!serde_err.to_string().is_empty()); +// assert!(!format!("{:?}", serde_err).is_empty()); +// } + +// #[test] +// fn save_errors() { +// // serde_json will raise an error as it has certain requirements on keys. +// let mut object = HashMap::, String>::new(); +// object.insert(Ok(()), String::from("string")); +// let serde_err = serde_json::to_string(&object); +// assert!(serde_err.is_err()); + +// let serde_err: SaveError = serde_err.unwrap_err().into(); +// assert!(!serde_err.to_string().is_empty()); +// assert!(!format!("{:?}", serde_err).is_empty()); +// } +// } -- 2.47.1 From eccc8a880f9d0cd432244911a7375988980d08fa Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 13:50:44 +0100 Subject: [PATCH 04/20] A nicer version compiles --- src/external/database/sql/backend.rs | 41 +++++++++++------ src/external/database/sql/mod.rs | 67 +++++++++++++--------------- 2 files changed, 59 insertions(+), 49 deletions(-) diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index 763d267..ecd4510 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -2,11 +2,11 @@ use std::path::PathBuf; -use rusqlite::{self, CachedStatement, Connection, Params, Statement}; +use rusqlite::{self, CachedStatement, Connection, Params, Statement, Transaction}; use crate::external::database::{ serde::serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, - sql::{Error, ISqlDatabaseBackend}, + sql::{Error, ISqlDatabaseBackend, ISqlTransactionBackend}, }; /// SQLite database backend that uses SQLite as the implementation. @@ -14,6 +14,10 @@ pub struct SqlDatabaseSqliteBackend { conn: Connection, } +pub struct SqlTransactionSqliteBackend<'conn> { + tx: Transaction<'conn>, +} + impl SqlDatabaseSqliteBackend { /// Create a [`SqlDatabaseSqliteBackend`] that will read/write to the provided database. pub fn new>(path: P) -> Result { @@ -21,15 +25,17 @@ impl SqlDatabaseSqliteBackend { conn: Connection::open(path.into()).map_err(|err| Error::OpenError(err.to_string()))?, }) } +} +impl<'conn> SqlTransactionSqliteBackend<'conn> { fn prepare(&self, sql: &str) -> Result { - self.conn + self.tx .prepare(sql) .map_err(|err| Error::StmtError(err.to_string())) } fn prepare_cached(&self, sql: &str) -> Result { - self.conn + self.tx .prepare_cached(sql) .map_err(|err| Error::StmtError(err.to_string())) } @@ -47,15 +53,22 @@ impl From for Error { } } -impl ISqlDatabaseBackend for SqlDatabaseSqliteBackend { - fn begin_transaction(&self) -> Result<(), Error> { - let mut stmt = self.prepare_cached("BEGIN TRANSACTION;")?; - Self::execute(&mut stmt, ()) - } +impl<'conn> ISqlDatabaseBackend<'conn> for SqlDatabaseSqliteBackend { + type Tx = SqlTransactionSqliteBackend<'conn>; - fn commit_transaction(&self) -> Result<(), Error> { - let mut stmt = self.prepare_cached("COMMIT;")?; - Self::execute(&mut stmt, ()) + fn transaction(&'conn mut self) -> Result { + self.conn + .transaction() + .map(|tx| SqlTransactionSqliteBackend { tx }) + .map_err(|err| Error::OpenError(err.to_string())) + } +} + +impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { + fn commit(self) -> Result<(), Error> { + self.tx + .commit() + .map_err(|err| Error::ExecError(err.to_string())) } fn create_database_metadata_table(&self) -> Result<(), Error> { @@ -85,7 +98,7 @@ impl ISqlDatabaseBackend for SqlDatabaseSqliteBackend { Self::execute(&mut stmt, ()) } - fn drop_artists_table(&self) -> Result<(),Error> { + fn drop_artists_table(&self) -> Result<(), Error> { let mut stmt = self.prepare_cached("DROP TABLE artists")?; Self::execute(&mut stmt, ()) } @@ -109,7 +122,7 @@ impl ISqlDatabaseBackend for SqlDatabaseSqliteBackend { Self::execute(&mut stmt, ()) } - fn drop_albums_table(&self) -> Result<(),Error> { + fn drop_albums_table(&self) -> Result<(), Error> { let mut stmt = self.prepare_cached("DROP TABLE albums")?; Self::execute(&mut stmt, ()) } diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 932c180..8a02eda 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -16,13 +16,18 @@ use crate::{ }; /// Trait for the SQL database backend. -#[cfg_attr(test, automock)] -pub trait ISqlDatabaseBackend { - /// Begin an SQL transaction. - fn begin_transaction(&self) -> Result<(), Error>; +pub trait ISqlDatabaseBackend<'conn> { + type Tx: ISqlTransactionBackend + 'conn; - /// Commit ongoing transaction. - fn commit_transaction(&self) -> Result<(), Error>; + /// Begin an SQL transaction. + fn transaction(&'conn mut self) -> Result; +} + +/// Trait for the SQL database backend. +#[cfg_attr(test, automock)] +pub trait ISqlTransactionBackend { + /// Commit transaction. + fn commit(self) -> Result<(), Error>; /// Create the database metadata table (if needed). fn create_database_metadata_table(&self) -> Result<(), Error>; @@ -95,65 +100,57 @@ pub struct SqlDatabase { backend: SDB, } -impl SqlDatabase { +impl ISqlDatabaseBackend<'c>> SqlDatabase { /// Create a new SQL database with the provided backend, e.g. /// [`backend::SqlDatabaseSqliteBackend`]. pub fn new(backend: SDB) -> Result { - let db = SqlDatabase { backend }; - db.begin_transaction()?; - db.create_tables()?; - db.commit_transaction()?; + let mut db = SqlDatabase { backend }; + let tx = db.backend.transaction()?; + Self::create_tables(&tx)?; + tx.commit()?; Ok(db) } - fn begin_transaction(&self) -> Result<(), Error> { - self.backend.begin_transaction() - } - - fn commit_transaction(&self) -> Result<(), Error> { - self.backend.commit_transaction() - } - - fn create_tables(&self) -> Result<(), Error> { - self.backend.create_database_metadata_table()?; - self.backend.create_artists_table()?; - self.backend.create_albums_table()?; + fn create_tables(tx: &Tx) -> Result<(), Error> { + tx.create_database_metadata_table()?; + tx.create_artists_table()?; + tx.create_albums_table()?; Ok(()) } - fn drop_tables(&self) -> Result<(), Error> { - self.backend.drop_database_metadata_table()?; - self.backend.drop_artists_table()?; - self.backend.drop_albums_table()?; + fn drop_tables(tx: &Tx) -> Result<(), Error> { + tx.drop_database_metadata_table()?; + tx.drop_artists_table()?; + tx.drop_albums_table()?; Ok(()) } } -impl IDatabase for SqlDatabase { +impl ISqlDatabaseBackend<'c>> IDatabase for SqlDatabase { fn load(&self) -> Result { Ok(vec![]) } fn save(&mut self, collection: &Collection) -> Result<(), SaveError> { let database: SerializeDatabase = collection.into(); - self.begin_transaction()?; + let tx = self.backend.transaction()?; - self.drop_tables()?; - self.create_tables()?; + Self::drop_tables(&tx)?; + Self::create_tables(&tx)?; - self.backend.insert_database_version(&database)?; + tx.insert_database_version(&database)?; match database { SerializeDatabase::V20250103(artists) => { for artist in artists.iter() { - self.backend.insert_artist(artist)?; + tx.insert_artist(artist)?; for album in artist.albums.iter() { - self.backend.insert_album(artist.name, album)?; + tx.insert_album(artist.name, album)?; } } } } - self.commit_transaction()?; + tx.commit()?; Ok(()) } } -- 2.47.1 From a9782b74ccf3e63802df488b1e84a2c2ae71d4e4 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 14:12:40 +0100 Subject: [PATCH 05/20] Add tests --- src/external/database/sql/mod.rs | 4 +- tests/database/mod.rs | 5 ++- tests/database/sql.rs | 66 ++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) create mode 100644 tests/database/sql.rs diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 8a02eda..4b1f198 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -100,7 +100,7 @@ pub struct SqlDatabase { backend: SDB, } -impl ISqlDatabaseBackend<'c>> SqlDatabase { +impl ISqlDatabaseBackend<'conn>> SqlDatabase { /// Create a new SQL database with the provided backend, e.g. /// [`backend::SqlDatabaseSqliteBackend`]. pub fn new(backend: SDB) -> Result { @@ -126,7 +126,7 @@ impl ISqlDatabaseBackend<'c>> SqlDatabase { } } -impl ISqlDatabaseBackend<'c>> IDatabase for SqlDatabase { +impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase { fn load(&self) -> Result { Ok(vec![]) } diff --git a/tests/database/mod.rs b/tests/database/mod.rs index 4e33b53..c088ad8 100644 --- a/tests/database/mod.rs +++ b/tests/database/mod.rs @@ -1,3 +1,4 @@ -#![cfg(feature = "database-json")] - +#[cfg(feature = "database-json")] pub mod json; +#[cfg(feature = "database-sqlite")] +pub mod sql; diff --git a/tests/database/sql.rs b/tests/database/sql.rs new file mode 100644 index 0000000..ccdedb9 --- /dev/null +++ b/tests/database/sql.rs @@ -0,0 +1,66 @@ +use std::{fs, path::PathBuf}; + +use once_cell::sync::Lazy; + +use musichoard::{ + external::database::sql::{backend::SqlDatabaseSqliteBackend, SqlDatabase}, + interface::database::IDatabase, +}; + +use crate::testlib::COLLECTION; + +pub static DATABASE_TEST_FILE: Lazy = + Lazy::new(|| fs::canonicalize("./tests/files/database/database.db").unwrap()); + +// fn expected() -> Collection { +// let mut expected = COLLECTION.to_owned(); +// for artist in expected.iter_mut() { +// for album in artist.albums.iter_mut() { +// album.tracks.clear(); +// } +// } +// expected +// } + +#[test] +fn save() { + // let file = NamedTempFile::new().unwrap(); + + let backend = SqlDatabaseSqliteBackend::new(&*DATABASE_TEST_FILE).unwrap(); + let mut database = SqlDatabase::new(backend).unwrap(); + + 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); +// let database = JsonDatabase::new(backend); + +// let read_data: Vec = database.load().unwrap(); + +// let expected = expected(); +// assert_eq!(read_data, expected); +// } + +// #[test] +// fn reverse() { +// 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 read_data: Vec = database.load().unwrap(); + +// // Album data is not saved into database. +// let expected = expected(); +// assert_eq!(read_data, expected); +// } -- 2.47.1 From 0fe6fe0be11c55a9262aed66269ef4150df01ab6 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 15:47:07 +0100 Subject: [PATCH 06/20] Complete implementation --- src/core/interface/database/mod.rs | 6 +- src/external/database/json/mod.rs | 2 +- src/external/database/serde/deserialize.rs | 28 +++--- src/external/database/serde/mod.rs | 2 +- src/external/database/sql/backend.rs | 98 +++++++++++++++++++-- src/external/database/sql/mod.rs | 40 ++++++++- src/external/musicbrainz/api/mod.rs | 2 +- tests/database/json.rs | 2 +- tests/database/sql.rs | 69 +++++++-------- tests/files/database/database.db | Bin 0 -> 28672 bytes 10 files changed, 185 insertions(+), 64 deletions(-) create mode 100644 tests/files/database/database.db diff --git a/src/core/interface/database/mod.rs b/src/core/interface/database/mod.rs index b60575c..3f9ab5f 100644 --- a/src/core/interface/database/mod.rs +++ b/src/core/interface/database/mod.rs @@ -11,7 +11,7 @@ use crate::core::collection::{self, Collection}; #[cfg_attr(test, automock)] pub trait IDatabase { /// Load collection from the database. - fn load(&self) -> Result; + fn load(&mut self) -> Result; /// Save collection to the database. fn save(&mut self, collection: &Collection) -> Result<(), SaveError>; @@ -21,7 +21,7 @@ pub trait IDatabase { pub struct NullDatabase; impl IDatabase for NullDatabase { - fn load(&self) -> Result { + fn load(&mut self) -> Result { Ok(vec![]) } @@ -100,7 +100,7 @@ mod tests { #[test] fn null_database_load() { - let database = NullDatabase; + let mut database = NullDatabase; assert!(database.load().unwrap().is_empty()); } diff --git a/src/external/database/json/mod.rs b/src/external/database/json/mod.rs index 0b7edf9..3d4709b 100644 --- a/src/external/database/json/mod.rs +++ b/src/external/database/json/mod.rs @@ -49,7 +49,7 @@ impl JsonDatabase { } impl IDatabase for JsonDatabase { - fn load(&self) -> Result { + fn load(&mut self) -> Result { let serialized = self.backend.read()?; let database: DeserializeDatabase = serde_json::from_str(&serialized)?; Ok(database.into()) diff --git a/src/external/database/serde/deserialize.rs b/src/external/database/serde/deserialize.rs index 182c9c6..1ce3da6 100644 --- a/src/external/database/serde/deserialize.rs +++ b/src/external/database/serde/deserialize.rs @@ -32,22 +32,22 @@ impl From for Collection { #[derive(Debug, Deserialize)] pub struct DeserializeArtist { - name: String, - sort: Option, - musicbrainz: DeserializeMbRefOption, - properties: HashMap>, - albums: Vec, + pub name: String, + pub sort: Option, + pub musicbrainz: DeserializeMbRefOption, + pub properties: HashMap>, + pub albums: Vec, } #[derive(Debug, Deserialize)] pub struct DeserializeAlbum { - title: String, - lib_id: SerdeAlbumLibId, - date: SerdeAlbumDate, - seq: u8, - musicbrainz: DeserializeMbRefOption, - primary_type: Option, - secondary_types: Vec, + pub title: String, + pub lib_id: SerdeAlbumLibId, + pub date: SerdeAlbumDate, + pub seq: u8, + pub musicbrainz: DeserializeMbRefOption, + pub primary_type: Option, + pub secondary_types: Vec, } #[derive(Debug, Deserialize)] @@ -110,6 +110,8 @@ impl<'de> Deserialize<'de> for DeserializeMbid { impl From for Artist { fn from(artist: DeserializeArtist) -> Self { + let mut albums: Vec = artist.albums.into_iter().map(Into::into).collect(); + albums.sort_unstable(); Artist { meta: ArtistMeta { id: ArtistId { @@ -121,7 +123,7 @@ impl From for Artist { properties: artist.properties, }, }, - albums: artist.albums.into_iter().map(Into::into).collect(), + albums, } } } diff --git a/src/external/database/serde/mod.rs b/src/external/database/serde/mod.rs index 6ccd8c4..9ca8585 100644 --- a/src/external/database/serde/mod.rs +++ b/src/external/database/serde/mod.rs @@ -1,5 +1,5 @@ //! Helper module for backends that can use serde for (de)serialisation. -mod common; +pub mod common; pub mod deserialize; pub mod serialize; diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index ecd4510..06fc99d 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -2,11 +2,20 @@ use std::path::PathBuf; -use rusqlite::{self, CachedStatement, Connection, Params, Statement, Transaction}; +use rusqlite::{ + self, types::FromSql, CachedStatement, Connection, Params, Row, Rows, Statement, Transaction, +}; -use crate::external::database::{ - serde::serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, - sql::{Error, ISqlDatabaseBackend, ISqlTransactionBackend}, +use crate::{ + collection::album::AlbumDate, + external::database::{ + serde::{ + common::SerdeAlbumDate, + deserialize::{DeserializeAlbum, DeserializeArtist, DeserializeDatabase}, + serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, + }, + sql::{Error, ISqlDatabaseBackend, ISqlTransactionBackend}, + }, }; /// SQLite database backend that uses SQLite as the implementation. @@ -45,6 +54,21 @@ impl<'conn> SqlTransactionSqliteBackend<'conn> { .map(|_| ()) .map_err(|err| Error::ExecError(err.to_string())) } + + fn query<'s, P: Params>(stmt: &'s mut Statement, params: P) -> Result, Error> { + stmt.query(params) + .map_err(|err| Error::ExecError(err.to_string())) + } + + fn next_row<'r, 's>(rows: &'r mut Rows<'s>) -> Result>, Error> { + rows.next() + .map_err(|err| Error::SerDeError(err.to_string())) + } + + fn get_value(row: &Row<'_>, idx: usize) -> Result { + row.get(idx) + .map_err(|err| Error::SerDeError(err.to_string())) + } } impl From for Error { @@ -116,7 +140,7 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { seq INT NOT NULL, primary_type JSON NOT NULL DEFAULT 'null', secondary_types JSON NOT NULL DEFAULT '[]', - FOREIGN KEY (artist_name) REFERENCES artists(name) + FOREIGN KEY (artist_name) REFERENCES artists(name) ON DELETE CASCADE ON UPDATE NO ACTION )", )?; Self::execute(&mut stmt, ()) @@ -138,6 +162,25 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { Self::execute(&mut stmt, ("version", version)) } + fn select_database_version<'a>(&self) -> Result { + let mut stmt = + self.prepare_cached("SELECT value FROM database_metadata WHERE name = 'version'")?; + let mut rows = Self::query(&mut stmt, ())?; + + match Self::next_row(&mut rows)? { + Some(row) => { + let version: String = Self::get_value(&row, 0)?; + match version.as_str() { + "V20250103" => Ok(DeserializeDatabase::V20250103(vec![])), + s @ _ => Err(Error::SerDeError(format!("unknown database version: {s}"))), + } + } + None => Err(Error::SerDeError(String::from( + "database version is missing", + ))), + } + } + fn insert_artist<'a>(&self, artist: &SerializeArtist<'a>) -> Result<(), Error> { let mut stmt = self.prepare_cached( "INSERT INTO artists (name, sort, mbid, properties) @@ -154,6 +197,24 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { ) } + fn select_all_artists(&self) -> Result, Error> { + let mut stmt = self.prepare_cached("SELECT name, sort, mbid, properties FROM artists")?; + let mut rows = Self::query(&mut stmt, ())?; + + let mut artists = vec![]; + while let Some(row) = Self::next_row(&mut rows)? { + artists.push(DeserializeArtist { + name: Self::get_value(row, 0)?, + sort: Self::get_value(row, 1)?, + musicbrainz: serde_json::from_str(&Self::get_value::(row, 2)?)?, + properties: serde_json::from_str(&Self::get_value::(row, 3)?)?, + albums: vec![], + }); + } + + Ok(artists) + } + fn insert_album<'a>(&self, artist_name: &str, album: &SerializeAlbum<'a>) -> Result<(), Error> { let mut stmt = self.prepare_cached( "INSERT INTO albums (title, lib_id, mbid, artist_name, @@ -176,4 +237,31 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { ), ) } + + fn select_artist_albums(&self, artist_name: &str) -> Result, Error> { + let mut stmt = self.prepare_cached( + "SELECT title, lib_id, year, month, day, seq, mbid, primary_type, secondary_types + FROM albums WHERE artist_name = ?1", + )?; + let mut rows = Self::query(&mut stmt, [artist_name])?; + + let mut albums = vec![]; + while let Some(row) = Self::next_row(&mut rows)? { + albums.push(DeserializeAlbum { + title: Self::get_value(row, 0)?, + lib_id: serde_json::from_str(&Self::get_value::(row, 1)?)?, + date: SerdeAlbumDate(AlbumDate::new( + Self::get_value(row, 2)?, + Self::get_value(row, 3)?, + Self::get_value(row, 4)?, + )), + seq: Self::get_value(row, 5)?, + musicbrainz: serde_json::from_str(&Self::get_value::(row, 6)?)?, + primary_type: serde_json::from_str(&Self::get_value::(row, 7)?)?, + secondary_types: serde_json::from_str(&Self::get_value::(row, 8)?)?, + }); + } + + Ok(albums) + } } diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 4b1f198..137ecee 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -12,7 +12,10 @@ use crate::{ collection::Collection, interface::database::{IDatabase, LoadError, SaveError}, }, - external::database::serde::serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, + external::database::serde::{ + deserialize::{DeserializeAlbum, DeserializeArtist, DeserializeDatabase}, + serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, + }, }; /// Trait for the SQL database backend. @@ -50,11 +53,20 @@ pub trait ISqlTransactionBackend { /// Set the database version. fn insert_database_version<'a>(&self, version: &SerializeDatabase<'a>) -> Result<(), Error>; + /// Get the database version. + fn select_database_version<'a>(&self) -> Result; + /// Insert an artist into the artist table. fn insert_artist<'a>(&self, artist: &SerializeArtist<'a>) -> Result<(), Error>; + /// Get all artists from the artist table. + fn select_all_artists(&self) -> Result, Error>; + /// Insert an artist into the artist table. fn insert_album<'a>(&self, artist_name: &str, album: &SerializeAlbum<'a>) -> Result<(), Error>; + + /// Get all albums from the album table. + fn select_artist_albums(&self, artist_name: &str) -> Result, Error>; } /// Errors for SQL database backend. @@ -95,6 +107,15 @@ impl From for SaveError { } } +impl From for LoadError { + fn from(value: Error) -> Self { + match value { + Error::SerDeError(s) => LoadError::SerDeError(s), + _ => LoadError::IoError(value.to_string()), + } + } +} + /// SQL database. pub struct SqlDatabase { backend: SDB, @@ -127,8 +148,21 @@ impl ISqlDatabaseBackend<'conn>> SqlDatabase { } impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase { - fn load(&self) -> Result { - Ok(vec![]) + fn load(&mut self) -> Result { + let tx = self.backend.transaction()?; + + let mut database = tx.select_database_version()?; + match database { + DeserializeDatabase::V20250103(ref mut coll) => { + coll.extend(tx.select_all_artists()?); + for artist in coll.iter_mut() { + artist.albums.extend(tx.select_artist_albums(&artist.name)?); + } + } + } + + tx.commit()?; + Ok(database.into()) } fn save(&mut self, collection: &Collection) -> Result<(), SaveError> { diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index bc3cc60..75c3c66 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -227,7 +227,7 @@ impl<'de> Deserialize<'de> for SerdeMbid { } #[derive(Debug, Clone)] -pub struct SerdeAlbumDate(AlbumDate); +pub struct SerdeAlbumDate(pub AlbumDate); impl From for AlbumDate { fn from(value: SerdeAlbumDate) -> Self { diff --git a/tests/database/json.rs b/tests/database/json.rs index da96d9e..9e13334 100644 --- a/tests/database/json.rs +++ b/tests/database/json.rs @@ -43,7 +43,7 @@ fn save() { #[test] fn load() { let backend = JsonDatabaseFileBackend::new(&*DATABASE_TEST_FILE); - let database = JsonDatabase::new(backend); + let mut database = JsonDatabase::new(backend); let read_data: Vec = database.load().unwrap(); diff --git a/tests/database/sql.rs b/tests/database/sql.rs index ccdedb9..ea7461a 100644 --- a/tests/database/sql.rs +++ b/tests/database/sql.rs @@ -3,64 +3,61 @@ use std::{fs, path::PathBuf}; use once_cell::sync::Lazy; use musichoard::{ + collection::{artist::Artist, Collection}, external::database::sql::{backend::SqlDatabaseSqliteBackend, SqlDatabase}, interface::database::IDatabase, }; +use tempfile::NamedTempFile; use crate::testlib::COLLECTION; pub static DATABASE_TEST_FILE: Lazy = Lazy::new(|| fs::canonicalize("./tests/files/database/database.db").unwrap()); -// fn expected() -> Collection { -// let mut expected = COLLECTION.to_owned(); -// for artist in expected.iter_mut() { -// for album in artist.albums.iter_mut() { -// album.tracks.clear(); -// } -// } -// expected -// } +fn expected() -> Collection { + let mut expected = COLLECTION.to_owned(); + for artist in expected.iter_mut() { + for album in artist.albums.iter_mut() { + album.tracks.clear(); + } + } + expected +} #[test] fn save() { - // let file = NamedTempFile::new().unwrap(); + let file = NamedTempFile::new().unwrap(); - let backend = SqlDatabaseSqliteBackend::new(&*DATABASE_TEST_FILE).unwrap(); + let backend = SqlDatabaseSqliteBackend::new(file.path()).unwrap(); let mut database = SqlDatabase::new(backend).unwrap(); 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); -// let database = JsonDatabase::new(backend); +#[test] +fn load() { + let backend = SqlDatabaseSqliteBackend::new(&*DATABASE_TEST_FILE).unwrap(); + let mut database = SqlDatabase::new(backend).unwrap(); -// let read_data: Vec = database.load().unwrap(); + let read_data: Vec = database.load().unwrap(); -// let expected = expected(); -// assert_eq!(read_data, expected); -// } + let expected = expected(); + assert_eq!(read_data, expected); +} -// #[test] -// fn reverse() { -// let file = NamedTempFile::new().unwrap(); +#[test] +fn reverse() { + let file = NamedTempFile::new().unwrap(); -// let backend = JsonDatabaseFileBackend::new(file.path()); -// let mut database = JsonDatabase::new(backend); + let backend = SqlDatabaseSqliteBackend::new(file.path()).unwrap(); + let mut database = SqlDatabase::new(backend).unwrap(); -// let write_data = COLLECTION.to_owned(); -// database.save(&write_data).unwrap(); -// let read_data: Vec = database.load().unwrap(); + let write_data = COLLECTION.to_owned(); + database.save(&write_data).unwrap(); + let read_data: Vec = database.load().unwrap(); -// // Album data is not saved into database. -// let expected = expected(); -// assert_eq!(read_data, expected); -// } + // Not all data is saved into database. + let expected = expected(); + assert_eq!(read_data, expected); +} diff --git a/tests/files/database/database.db b/tests/files/database/database.db new file mode 100644 index 0000000000000000000000000000000000000000..9fe47ada8d16334914faad64d9c5f70de30597aa GIT binary patch literal 28672 zcmeI5Uu@e%9LMdtO}d4~Y!ig2LbY6#ZDMZXI8N>I0BPsJSl5oG9gM1qF7~B0b?nS` zT1r*1bqI;Ky`eoI@x%iU6B6PD0Yc)>8(JrjKtkf7uY2JI!4u*fr(V~jMMNM0`kmtV zeCOZ!zMnf=`d#jv(hF0jkGZPj){xIh)WZ}_Q_pZ5MNtW|#mTlCo+gpr-2<|x_rwp1 zCaBTXw+7hXs90i@O8(A%H1JLGPGWT6-GdAu9S8scAOHk_01yBIKmZ8*dkDPY^u<%@ zH2s$9ql$%*RcX|`@H;*^s}<)oZmu{nrEy^zcj_p&Klr9^VQx-)X^xwoA=~`a6nAO% z{Kewz74Cv|*DAB4zmSXL)M z9@ZV(*tPZ!GGAEiWPEOBRy+UfbkJtpsXfq6bFkx`pH-<^>mu1OxwV(dRHyttd&v2cbdoL zu$z^|aNm3Vbtaw~9Hei|huyQ;lwPY6eGhifFt1Y&ZEyNN>7btD`mG*KdUJaHi& zU8jyoHSvKhv$1({A1e3?;>-!U^u)#fM;VcxH4V)9%b1%omzI6ow3p&6 zGfdCh6&ItGU>#BHVP=HBgve?K?vjO;8o2K%W`o|j_MVNr*D&`A_Iz%|{o+H5MAn=$ zT;1`%ytRZllhc>i&{tcCvk|vu;uVwF^d4o>bjd=tiI~S2nZEqR`!=`W;+<00KY&2mk>f00e*l5cmrOuD{e5 z8=_XWiWkW;V#_jhw85908s^9Of?Um4v$`zcs)~iYn$-n_u_UNj6^|iYsN_(E-`wCY zHat_GXpmP0xcvA6zwG;UZ#5yt!&LdhtVpwcdp&?xCydix`u7PLsFz$&*^zXE+D~>RaM9%Q4uOeu1drg@_C#S zRYljk*PT;EQC12{cgmNWQUp&3mVO694kz$3(`;ProJ-zr;1S<05-A}yD@w8?RCV1T zBCCdgjI1cAdRD6DWwAoqyL*vJt{^LllFfG2yw%;>{)xDrP@HBpRPj5mt?i-WwvN-Wk@U)q;q9Au zuWf(4{n_@X+aDC&7014Re^wAOa)q2B$TF@9`P>*16iFWws$;U0RSWrQPRs==Cy;IE zs8-*5hoNRsHOJGPwb4rJgy!8Ai0bZ+&M8tpugHqn6_>C^he8Q{|Lr%oJU?F84F519@*?AR_+s!k7W@J~Wv*g= zaUz}&cTJDA&s~p7lG{r9e7B@UEtVJ_PD{j61lm@gJjNCi@a2tzsTI zws(F!Um~+oEbKcrAIj-iPVm1E3yVv=iH%54mvic)dP}W(NA7j}ZdBU>lSoGw3Mb%4 zcDa^a Date: Sat, 11 Jan 2025 16:02:47 +0100 Subject: [PATCH 07/20] Clippy lints --- Cargo.toml | 2 +- src/external/database/sql/backend.rs | 14 +++++++------- src/external/database/sql/mod.rs | 5 ++++- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5d159f8..a47e9c6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -33,7 +33,7 @@ tempfile = "3.15.0" [features] default = ["database-json", "library-beets"] bin = ["structopt"] -database-sqlite = ["rusqlite"] +database-sqlite = ["rusqlite", "serde", "serde_json"] database-json = ["serde", "serde_json"] library-beets = [] library-beets-ssh = ["openssh", "tokio"] diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index 06fc99d..41efd1c 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -36,7 +36,7 @@ impl SqlDatabaseSqliteBackend { } } -impl<'conn> SqlTransactionSqliteBackend<'conn> { +impl SqlTransactionSqliteBackend<'_> { fn prepare(&self, sql: &str) -> Result { self.tx .prepare(sql) @@ -88,7 +88,7 @@ impl<'conn> ISqlDatabaseBackend<'conn> for SqlDatabaseSqliteBackend { } } -impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { +impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { fn commit(self) -> Result<(), Error> { self.tx .commit() @@ -151,7 +151,7 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { Self::execute(&mut stmt, ()) } - fn insert_database_version<'a>(&self, version: &SerializeDatabase<'a>) -> Result<(), Error> { + fn insert_database_version(&self, version: &SerializeDatabase<'_>) -> Result<(), Error> { let mut stmt = self.prepare_cached( "INSERT INTO database_metadata (name, value) VALUES (?1, ?2)", @@ -169,10 +169,10 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { match Self::next_row(&mut rows)? { Some(row) => { - let version: String = Self::get_value(&row, 0)?; + let version: String = Self::get_value(row, 0)?; match version.as_str() { "V20250103" => Ok(DeserializeDatabase::V20250103(vec![])), - s @ _ => Err(Error::SerDeError(format!("unknown database version: {s}"))), + s => Err(Error::SerDeError(format!("unknown database version: {s}"))), } } None => Err(Error::SerDeError(String::from( @@ -181,7 +181,7 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { } } - fn insert_artist<'a>(&self, artist: &SerializeArtist<'a>) -> Result<(), Error> { + fn insert_artist(&self, artist: &SerializeArtist<'_>) -> Result<(), Error> { let mut stmt = self.prepare_cached( "INSERT INTO artists (name, sort, mbid, properties) VALUES (?1, ?2, ?3, ?4)", @@ -215,7 +215,7 @@ impl<'conn> ISqlTransactionBackend for SqlTransactionSqliteBackend<'conn> { Ok(artists) } - fn insert_album<'a>(&self, artist_name: &str, album: &SerializeAlbum<'a>) -> Result<(), Error> { + fn insert_album(&self, artist_name: &str, album: &SerializeAlbum<'_>) -> Result<(), Error> { let mut stmt = self.prepare_cached( "INSERT INTO albums (title, lib_id, mbid, artist_name, year, month, day, seq, primary_type, secondary_types) diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 137ecee..93b4bb7 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -51,18 +51,21 @@ pub trait ISqlTransactionBackend { fn drop_albums_table(&self) -> Result<(), Error>; /// Set the database version. + #[allow(clippy::needless_lifetimes)] // Conflicts with automock. fn insert_database_version<'a>(&self, version: &SerializeDatabase<'a>) -> Result<(), Error>; /// Get the database version. - fn select_database_version<'a>(&self) -> Result; + fn select_database_version(&self) -> Result; /// Insert an artist into the artist table. + #[allow(clippy::needless_lifetimes)] // Conflicts with automock. fn insert_artist<'a>(&self, artist: &SerializeArtist<'a>) -> Result<(), Error>; /// Get all artists from the artist table. fn select_all_artists(&self) -> Result, Error>; /// Insert an artist into the artist table. + #[allow(clippy::needless_lifetimes)] // Conflicts with automock. fn insert_album<'a>(&self, artist_name: &str, album: &SerializeAlbum<'a>) -> Result<(), Error>; /// Get all albums from the album table. -- 2.47.1 From 8c83c6fac6756f0bc0e9c48acac5de75a904cc64 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 16:15:57 +0100 Subject: [PATCH 08/20] Clean up some code --- src/external/database/mod.rs | 2 +- src/external/database/serde/common.rs | 2 ++ src/external/database/sql/backend.rs | 21 ++++++++------------- src/external/musicbrainz/api/mod.rs | 2 +- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/external/database/mod.rs b/src/external/database/mod.rs index 72a8931..ceca477 100644 --- a/src/external/database/mod.rs +++ b/src/external/database/mod.rs @@ -3,5 +3,5 @@ pub mod json; #[cfg(feature = "database-sqlite")] pub mod sql; -#[cfg(feature = "database-json")] +#[cfg(any(feature = "database-json", feature = "database-sqlite"))] mod serde; diff --git a/src/external/database/serde/common.rs b/src/external/database/serde/common.rs index e46b3de..2dfea05 100644 --- a/src/external/database/serde/common.rs +++ b/src/external/database/serde/common.rs @@ -5,6 +5,8 @@ use crate::core::collection::{ musicbrainz::MbRefOption, }; +pub const V20250103: &str = "V20250103"; + #[derive(Debug, Deserialize, Serialize)] #[serde(remote = "AlbumLibId")] pub enum AlbumLibIdDef { diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index 41efd1c..3273751 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -1,4 +1,4 @@ -//! Module for storing MusicHoard data in a JSON file database. +//! Module for storing MusicHoard data in a SQLite database. use std::path::PathBuf; @@ -10,7 +10,7 @@ use crate::{ collection::album::AlbumDate, external::database::{ serde::{ - common::SerdeAlbumDate, + common::{SerdeAlbumDate, V20250103}, deserialize::{DeserializeAlbum, DeserializeArtist, DeserializeDatabase}, serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, }, @@ -157,7 +157,7 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { VALUES (?1, ?2)", )?; let version = match version { - SerializeDatabase::V20250103(_) => "V20250103", + SerializeDatabase::V20250103(_) => V20250103, }; Self::execute(&mut stmt, ("version", version)) } @@ -168,16 +168,11 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { let mut rows = Self::query(&mut stmt, ())?; match Self::next_row(&mut rows)? { - Some(row) => { - let version: String = Self::get_value(row, 0)?; - match version.as_str() { - "V20250103" => Ok(DeserializeDatabase::V20250103(vec![])), - s => Err(Error::SerDeError(format!("unknown database version: {s}"))), - } - } - None => Err(Error::SerDeError(String::from( - "database version is missing", - ))), + Some(row) => match Self::get_value::(row, 0)?.as_str() { + V20250103 => Ok(DeserializeDatabase::V20250103(vec![])), + s => Err(Error::SerDeError(format!("unknown database version: {s}"))), + }, + None => Err(Error::SerDeError(String::from("missing database version"))), } } diff --git a/src/external/musicbrainz/api/mod.rs b/src/external/musicbrainz/api/mod.rs index 75c3c66..bc3cc60 100644 --- a/src/external/musicbrainz/api/mod.rs +++ b/src/external/musicbrainz/api/mod.rs @@ -227,7 +227,7 @@ impl<'de> Deserialize<'de> for SerdeMbid { } #[derive(Debug, Clone)] -pub struct SerdeAlbumDate(pub AlbumDate); +pub struct SerdeAlbumDate(AlbumDate); impl From for AlbumDate { fn from(value: SerdeAlbumDate) -> Self { -- 2.47.1 From 65299269c4d79142c1080bd4a1fa32c528be6119 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 16:38:10 +0100 Subject: [PATCH 09/20] Move logic to mod from backend --- src/external/database/sql/backend.rs | 23 ++++++++--------------- src/external/database/sql/mod.rs | 27 ++++++++++++++++++--------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index 3273751..ad70b86 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -10,9 +10,9 @@ use crate::{ collection::album::AlbumDate, external::database::{ serde::{ - common::{SerdeAlbumDate, V20250103}, - deserialize::{DeserializeAlbum, DeserializeArtist, DeserializeDatabase}, - serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, + common::SerdeAlbumDate, + deserialize::{DeserializeAlbum, DeserializeArtist}, + serialize::{SerializeAlbum, SerializeArtist}, }, sql::{Error, ISqlDatabaseBackend, ISqlTransactionBackend}, }, @@ -151,29 +151,22 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { Self::execute(&mut stmt, ()) } - fn insert_database_version(&self, version: &SerializeDatabase<'_>) -> Result<(), Error> { + fn insert_database_version(&self, version: &str) -> Result<(), Error> { let mut stmt = self.prepare_cached( "INSERT INTO database_metadata (name, value) VALUES (?1, ?2)", )?; - let version = match version { - SerializeDatabase::V20250103(_) => V20250103, - }; Self::execute(&mut stmt, ("version", version)) } - fn select_database_version<'a>(&self) -> Result { + fn select_database_version<'a>(&self) -> Result, Error> { let mut stmt = self.prepare_cached("SELECT value FROM database_metadata WHERE name = 'version'")?; let mut rows = Self::query(&mut stmt, ())?; - match Self::next_row(&mut rows)? { - Some(row) => match Self::get_value::(row, 0)?.as_str() { - V20250103 => Ok(DeserializeDatabase::V20250103(vec![])), - s => Err(Error::SerDeError(format!("unknown database version: {s}"))), - }, - None => Err(Error::SerDeError(String::from("missing database version"))), - } + Self::next_row(&mut rows)? + .map(|row| Self::get_value(row, 0)) + .transpose() } fn insert_artist(&self, artist: &SerializeArtist<'_>) -> Result<(), Error> { diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 93b4bb7..0f2b445 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -13,6 +13,7 @@ use crate::{ interface::database::{IDatabase, LoadError, SaveError}, }, external::database::serde::{ + common::V20250103, deserialize::{DeserializeAlbum, DeserializeArtist, DeserializeDatabase}, serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, }, @@ -51,11 +52,10 @@ pub trait ISqlTransactionBackend { fn drop_albums_table(&self) -> Result<(), Error>; /// Set the database version. - #[allow(clippy::needless_lifetimes)] // Conflicts with automock. - fn insert_database_version<'a>(&self, version: &SerializeDatabase<'a>) -> Result<(), Error>; + fn insert_database_version(&self, version: &str) -> Result<(), Error>; /// Get the database version. - fn select_database_version(&self) -> Result; + fn select_database_version(&self) -> Result, Error>; /// Insert an artist into the artist table. #[allow(clippy::needless_lifetimes)] // Conflicts with automock. @@ -154,15 +154,24 @@ impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase fn load(&mut self) -> Result { let tx = self.backend.transaction()?; - let mut database = tx.select_database_version()?; - match database { - DeserializeDatabase::V20250103(ref mut coll) => { - coll.extend(tx.select_all_artists()?); + let version = tx + .select_database_version()? + .ok_or_else(|| LoadError::SerDeError(String::from("missing database version")))?; + + let database = match version.as_str() { + V20250103 => { + let mut coll = tx.select_all_artists()?; for artist in coll.iter_mut() { artist.albums.extend(tx.select_artist_albums(&artist.name)?); } + DeserializeDatabase::V20250103(coll) } - } + s => { + return Err(LoadError::SerDeError(format!( + "unknown database version: {s}" + ))) + } + }; tx.commit()?; Ok(database.into()) @@ -175,9 +184,9 @@ impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase Self::drop_tables(&tx)?; Self::create_tables(&tx)?; - tx.insert_database_version(&database)?; match database { SerializeDatabase::V20250103(artists) => { + tx.insert_database_version(V20250103)?; for artist in artists.iter() { tx.insert_artist(artist)?; for album in artist.albums.iter() { -- 2.47.1 From 7b6cb56066006d9b9611993bc36569058cf65b31 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 16:40:18 +0100 Subject: [PATCH 10/20] fix --- src/external/database/sql/mod.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 0f2b445..c8ded2f 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -156,7 +156,7 @@ impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase let version = tx .select_database_version()? - .ok_or_else(|| LoadError::SerDeError(String::from("missing database version")))?; + .ok_or_else(|| Error::SerDeError(String::from("missing database version")))?; let database = match version.as_str() { V20250103 => { @@ -164,14 +164,10 @@ impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase for artist in coll.iter_mut() { artist.albums.extend(tx.select_artist_albums(&artist.name)?); } - DeserializeDatabase::V20250103(coll) + Ok(DeserializeDatabase::V20250103(coll)) } - s => { - return Err(LoadError::SerDeError(format!( - "unknown database version: {s}" - ))) - } - }; + s => Err(Error::SerDeError(format!("unknown database version: {s}"))), + }?; tx.commit()?; Ok(database.into()) -- 2.47.1 From 03d950109be899df8fd1b6058bcef922f11fbdb0 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 17:09:30 +0100 Subject: [PATCH 11/20] Fix compilation --- src/external/database/serde/common.rs | 2 -- src/external/database/sql/mod.rs | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/external/database/serde/common.rs b/src/external/database/serde/common.rs index 2dfea05..e46b3de 100644 --- a/src/external/database/serde/common.rs +++ b/src/external/database/serde/common.rs @@ -5,8 +5,6 @@ use crate::core::collection::{ musicbrainz::MbRefOption, }; -pub const V20250103: &str = "V20250103"; - #[derive(Debug, Deserialize, Serialize)] #[serde(remote = "AlbumLibId")] pub enum AlbumLibIdDef { diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index c8ded2f..f215a5a 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -13,12 +13,13 @@ use crate::{ interface::database::{IDatabase, LoadError, SaveError}, }, external::database::serde::{ - common::V20250103, deserialize::{DeserializeAlbum, DeserializeArtist, DeserializeDatabase}, serialize::{SerializeAlbum, SerializeArtist, SerializeDatabase}, }, }; +const V20250103: &str = "V20250103"; + /// Trait for the SQL database backend. pub trait ISqlDatabaseBackend<'conn> { type Tx: ISqlTransactionBackend + 'conn; -- 2.47.1 From e24f77689614510344947f58c00f53acfd84c53a Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sat, 11 Jan 2025 20:44:47 +0100 Subject: [PATCH 12/20] Succinct unit tests --- src/external/database/serde/common.rs | 8 +- src/external/database/serde/serialize.rs | 8 +- src/external/database/sql/mod.rs | 249 +++++++++++++++-------- 3 files changed, 174 insertions(+), 91 deletions(-) diff --git a/src/external/database/serde/common.rs b/src/external/database/serde/common.rs index e46b3de..07f085b 100644 --- a/src/external/database/serde/common.rs +++ b/src/external/database/serde/common.rs @@ -13,7 +13,7 @@ pub enum AlbumLibIdDef { None, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumLibId(#[serde(with = "AlbumLibIdDef")] AlbumLibId); impl From for AlbumLibId { @@ -36,7 +36,7 @@ pub struct AlbumDateDef { day: Option, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumDate(#[serde(with = "AlbumDateDef")] pub AlbumDate); impl From for AlbumDate { @@ -69,7 +69,7 @@ pub enum AlbumPrimaryTypeDef { Other, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumPrimaryType(#[serde(with = "AlbumPrimaryTypeDef")] AlbumPrimaryType); impl From for AlbumPrimaryType { @@ -101,7 +101,7 @@ pub enum AlbumSecondaryTypeDef { FieldRecording, } -#[derive(Debug, Deserialize, Serialize)] +#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumSecondaryType(#[serde(with = "AlbumSecondaryTypeDef")] AlbumSecondaryType); impl From for AlbumSecondaryType { diff --git a/src/external/database/serde/serialize.rs b/src/external/database/serde/serialize.rs index aa9b336..197fc71 100644 --- a/src/external/database/serde/serialize.rs +++ b/src/external/database/serde/serialize.rs @@ -22,7 +22,7 @@ impl<'a> From<&'a Collection> for SerializeDatabase<'a> { } } -#[derive(Debug, Serialize)] +#[derive(Debug, Serialize, PartialEq, Eq)] pub struct SerializeArtist<'a> { pub name: &'a str, pub sort: Option<&'a str>, @@ -31,7 +31,7 @@ pub struct SerializeArtist<'a> { pub albums: Vec>, } -#[derive(Debug, Serialize)] +#[derive(Debug, Serialize, PartialEq, Eq)] pub struct SerializeAlbum<'a> { pub title: &'a str, pub lib_id: SerdeAlbumLibId, @@ -42,12 +42,12 @@ pub struct SerializeAlbum<'a> { pub secondary_types: Vec, } -#[derive(Debug, Serialize)] +#[derive(Debug, Serialize, PartialEq, Eq)] pub struct SerializeMbRefOption<'a>( #[serde(with = "MbRefOptionDef")] MbRefOption>, ); -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] pub struct SerializeMbid<'a>(&'a Mbid); impl<'a, T: IMusicBrainzRef> From<&'a MbRefOption> for SerializeMbRefOption<'a> { diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index f215a5a..d442002 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -144,9 +144,9 @@ impl ISqlDatabaseBackend<'conn>> SqlDatabase { } fn drop_tables(tx: &Tx) -> Result<(), Error> { - tx.drop_database_metadata_table()?; - tx.drop_artists_table()?; tx.drop_albums_table()?; + tx.drop_artists_table()?; + tx.drop_database_metadata_table()?; Ok(()) } } @@ -201,103 +201,186 @@ impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase // #[cfg(test)] // pub mod testmod; -// #[cfg(test)] -// mod tests { -// use std::collections::HashMap; +#[cfg(test)] +mod tests { + // use std::collections::HashMap; -// use mockall::predicate; + use std::collections::VecDeque; -// use crate::core::{ -// collection::{artist::Artist, Collection}, -// testmod::FULL_COLLECTION, -// }; + use mockall::{predicate, Sequence}; -// use super::*; -// use testmod::DATABASE_JSON; + use crate::core::{ + // collection::{artist::Artist, Collection}, + testmod::FULL_COLLECTION, + }; -// fn expected() -> Collection { -// let mut expected = FULL_COLLECTION.to_owned(); -// for artist in expected.iter_mut() { -// for album in artist.albums.iter_mut() { -// album.tracks.clear(); -// } -// } -// expected -// } + use super::*; -// #[test] -// fn save() { -// let write_data = FULL_COLLECTION.to_owned(); -// let input = DATABASE_JSON.to_owned(); + // fn expected() -> Collection { + // let mut expected = FULL_COLLECTION.to_owned(); + // for artist in expected.iter_mut() { + // for album in artist.albums.iter_mut() { + // album.tracks.clear(); + // } + // } + // expected + // } -// let mut backend = MockISqlDatabaseBackend::new(); -// backend -// .expect_write() -// .with(predicate::eq(input)) -// .times(1) -// .return_once(|_| Ok(())); + struct SqlDatabaseTestBackend { + pub txs: VecDeque, + } -// SqlDatabase::new(backend).save(&write_data).unwrap(); -// } + impl SqlDatabaseTestBackend { + fn new(txs: VecDeque) -> Self { + SqlDatabaseTestBackend { txs } + } + } -// #[test] -// fn load() { -// let expected = expected(); -// let result = Ok(DATABASE_JSON.to_owned()); -// eprintln!("{DATABASE_JSON}"); + impl ISqlDatabaseBackend<'_> for SqlDatabaseTestBackend { + type Tx = MockISqlTransactionBackend; -// let mut backend = MockISqlDatabaseBackend::new(); -// backend.expect_read().times(1).return_once(|| result); + fn transaction(&mut self) -> Result { + Ok(self.txs.pop_front().unwrap()) + } + } -// let read_data: Vec = SqlDatabase::new(backend).load().unwrap(); + macro_rules! then { + ($tx:ident, $seq:ident, $expect:ident) => { + $tx.$expect().times(1).in_sequence(&mut $seq) + }; + } -// assert_eq!(read_data, expected); -// } + macro_rules! then0 { + ($tx:ident, $seq:ident, $expect:ident) => { + then!($tx, $seq, $expect).return_once(|| Ok(())) + }; + } -// #[test] -// fn reverse() { -// let input = DATABASE_JSON.to_owned(); -// let result = Ok(input.clone()); + macro_rules! then1 { + ($tx:ident, $seq:ident, $expect:ident) => { + then!($tx, $seq, $expect).return_once(|_| Ok(())) + }; + } -// let mut backend = MockISqlDatabaseBackend::new(); -// backend -// .expect_write() -// .with(predicate::eq(input)) -// .times(1) -// .return_once(|_| Ok(())); -// backend.expect_read().times(1).return_once(|| result); -// let mut database = SqlDatabase::new(backend); + macro_rules! then2 { + ($tx:ident, $seq:ident, $expect:ident) => { + then!($tx, $seq, $expect).return_once(|_, _| Ok(())) + }; + } -// let write_data = FULL_COLLECTION.to_owned(); -// database.save(&write_data).unwrap(); -// let read_data: Vec = database.load().unwrap(); + macro_rules! expect_create { + ($tx:ident, $seq:ident) => { + let mut seq = Sequence::new(); + then0!($tx, seq, expect_create_database_metadata_table); + then0!($tx, seq, expect_create_artists_table); + then0!($tx, seq, expect_create_albums_table); + }; + } -// // Album information is not saved to disk. -// let expected = expected(); -// assert_eq!(read_data, expected); -// } + macro_rules! expect_drop { + ($tx:ident, $seq:ident) => { + let mut seq = Sequence::new(); + then0!($tx, seq, expect_drop_albums_table); + then0!($tx, seq, expect_drop_artists_table); + then0!($tx, seq, expect_drop_database_metadata_table); + }; + } -// #[test] -// fn load_errors() { -// let json = String::from(""); -// let serde_err = serde_json::from_str::(&json); -// assert!(serde_err.is_err()); + fn database(txs: VecDeque) -> SqlDatabase { + let mut backend = SqlDatabaseTestBackend::new(txs); + let mut tx = MockISqlTransactionBackend::new(); -// let serde_err: LoadError = serde_err.unwrap_err().into(); -// assert!(!serde_err.to_string().is_empty()); -// assert!(!format!("{:?}", serde_err).is_empty()); -// } + let mut seq = Sequence::new(); + expect_create!(tx, seq); + then0!(tx, seq, expect_commit); -// #[test] -// fn save_errors() { -// // serde_json will raise an error as it has certain requirements on keys. -// let mut object = HashMap::, String>::new(); -// object.insert(Ok(()), String::from("string")); -// let serde_err = serde_json::to_string(&object); -// assert!(serde_err.is_err()); + backend.txs.push_front(tx); + SqlDatabase::new(backend).unwrap() + } -// let serde_err: SaveError = serde_err.unwrap_err().into(); -// assert!(!serde_err.to_string().is_empty()); -// assert!(!format!("{:?}", serde_err).is_empty()); -// } -// } + #[test] + fn save() { + let write_data = FULL_COLLECTION.to_owned(); + + let mut tx = MockISqlTransactionBackend::new(); + + let mut seq = Sequence::new(); + expect_drop!(tx, seq); + expect_create!(tx, seq); + then1!(tx, seq, expect_insert_database_version).with(predicate::eq(V20250103)); + for artist in write_data.iter() { + let ac = artist.clone(); + then1!(tx, seq, expect_insert_artist) + .withf(move |a| a == &Into::::into(&ac)); + for album in artist.albums.iter() { + let (nc, ac) = (artist.meta.id.name.clone(), album.clone()); + then2!(tx, seq, expect_insert_album) + .withf(move |n, a| n == nc && a == &Into::::into(&ac)); + } + } + then0!(tx, seq, expect_commit); + + assert!(database(VecDeque::from([tx])).save(&write_data).is_ok()); + } + + // #[test] + // fn load() { + // let expected = expected(); + // let result = Ok(DATABASE_JSON.to_owned()); + // eprintln!("{DATABASE_JSON}"); + + // let mut backend = MockISqlDatabaseBackend::new(); + // backend.expect_read().times(1).return_once(|| result); + + // let read_data: Vec = SqlDatabase::new(backend).load().unwrap(); + + // assert_eq!(read_data, expected); + // } + + // #[test] + // fn reverse() { + // let input = DATABASE_JSON.to_owned(); + // let result = Ok(input.clone()); + + // let mut backend = MockISqlDatabaseBackend::new(); + // backend + // .expect_write() + // .with(predicate::eq(input)) + // .times(1) + // .return_once(|_| Ok(())); + // backend.expect_read().times(1).return_once(|| result); + // let mut database = SqlDatabase::new(backend); + + // let write_data = FULL_COLLECTION.to_owned(); + // database.save(&write_data).unwrap(); + // let read_data: Vec = database.load().unwrap(); + + // // 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::(&json); + // assert!(serde_err.is_err()); + + // let serde_err: LoadError = serde_err.unwrap_err().into(); + // assert!(!serde_err.to_string().is_empty()); + // assert!(!format!("{:?}", serde_err).is_empty()); + // } + + // #[test] + // fn save_errors() { + // // serde_json will raise an error as it has certain requirements on keys. + // let mut object = HashMap::, String>::new(); + // object.insert(Ok(()), String::from("string")); + // let serde_err = serde_json::to_string(&object); + // assert!(serde_err.is_err()); + + // let serde_err: SaveError = serde_err.unwrap_err().into(); + // assert!(!serde_err.to_string().is_empty()); + // assert!(!format!("{:?}", serde_err).is_empty()); + // } +} -- 2.47.1 From 634a17c2bf8031a3d635c834466e067a07d84c7b Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 07:43:46 +0100 Subject: [PATCH 13/20] Add load test --- src/external/database/serde/common.rs | 8 +-- src/external/database/serde/deserialize.rs | 6 +- src/external/database/sql/backend.rs | 58 ++++++++++------ src/external/database/sql/mod.rs | 77 +++++++++++++++------- 4 files changed, 98 insertions(+), 51 deletions(-) diff --git a/src/external/database/serde/common.rs b/src/external/database/serde/common.rs index 07f085b..8a2207d 100644 --- a/src/external/database/serde/common.rs +++ b/src/external/database/serde/common.rs @@ -13,7 +13,7 @@ pub enum AlbumLibIdDef { None, } -#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumLibId(#[serde(with = "AlbumLibIdDef")] AlbumLibId); impl From for AlbumLibId { @@ -36,7 +36,7 @@ pub struct AlbumDateDef { day: Option, } -#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumDate(#[serde(with = "AlbumDateDef")] pub AlbumDate); impl From for AlbumDate { @@ -69,7 +69,7 @@ pub enum AlbumPrimaryTypeDef { Other, } -#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumPrimaryType(#[serde(with = "AlbumPrimaryTypeDef")] AlbumPrimaryType); impl From for AlbumPrimaryType { @@ -101,7 +101,7 @@ pub enum AlbumSecondaryTypeDef { FieldRecording, } -#[derive(Debug, Deserialize, Serialize, PartialEq, Eq)] +#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] pub struct SerdeAlbumSecondaryType(#[serde(with = "AlbumSecondaryTypeDef")] AlbumSecondaryType); impl From for AlbumSecondaryType { diff --git a/src/external/database/serde/deserialize.rs b/src/external/database/serde/deserialize.rs index 1ce3da6..bd1db1a 100644 --- a/src/external/database/serde/deserialize.rs +++ b/src/external/database/serde/deserialize.rs @@ -30,7 +30,7 @@ impl From for Collection { } } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct DeserializeArtist { pub name: String, pub sort: Option, @@ -39,7 +39,7 @@ pub struct DeserializeArtist { pub albums: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct DeserializeAlbum { pub title: String, pub lib_id: SerdeAlbumLibId, @@ -50,7 +50,7 @@ pub struct DeserializeAlbum { pub secondary_types: Vec, } -#[derive(Debug, Deserialize)] +#[derive(Clone, Debug, Deserialize)] pub struct DeserializeMbRefOption(#[serde(with = "MbRefOptionDef")] MbRefOption); #[derive(Clone, Debug)] diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index ad70b86..9051ae9 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -191,13 +191,7 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { let mut artists = vec![]; while let Some(row) = Self::next_row(&mut rows)? { - artists.push(DeserializeArtist { - name: Self::get_value(row, 0)?, - sort: Self::get_value(row, 1)?, - musicbrainz: serde_json::from_str(&Self::get_value::(row, 2)?)?, - properties: serde_json::from_str(&Self::get_value::(row, 3)?)?, - albums: vec![], - }); + artists.push(row.try_into()?); } Ok(artists) @@ -235,21 +229,45 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { let mut albums = vec![]; while let Some(row) = Self::next_row(&mut rows)? { - albums.push(DeserializeAlbum { - title: Self::get_value(row, 0)?, - lib_id: serde_json::from_str(&Self::get_value::(row, 1)?)?, - date: SerdeAlbumDate(AlbumDate::new( - Self::get_value(row, 2)?, - Self::get_value(row, 3)?, - Self::get_value(row, 4)?, - )), - seq: Self::get_value(row, 5)?, - musicbrainz: serde_json::from_str(&Self::get_value::(row, 6)?)?, - primary_type: serde_json::from_str(&Self::get_value::(row, 7)?)?, - secondary_types: serde_json::from_str(&Self::get_value::(row, 8)?)?, - }); + albums.push(row.try_into()?); } Ok(albums) } } + +impl TryFrom<&Row<'_>> for DeserializeArtist { + type Error = Error; + + fn try_from(row: &Row<'_>) -> Result { + type Backend<'a> = SqlTransactionSqliteBackend<'a>; + Ok(DeserializeArtist { + name: Backend::get_value(row, 0)?, + sort: Backend::get_value(row, 1)?, + musicbrainz: serde_json::from_str(&Backend::get_value::(row, 2)?)?, + properties: serde_json::from_str(&Backend::get_value::(row, 3)?)?, + albums: vec![], + }) + } +} + +impl TryFrom<&Row<'_>> for DeserializeAlbum { + type Error = Error; + + fn try_from(row: &Row<'_>) -> Result { + type Backend<'a> = SqlTransactionSqliteBackend<'a>; + Ok(DeserializeAlbum { + title: Backend::get_value(row, 0)?, + lib_id: serde_json::from_str(&Backend::get_value::(row, 1)?)?, + date: SerdeAlbumDate(AlbumDate::new( + Backend::get_value(row, 2)?, + Backend::get_value(row, 3)?, + Backend::get_value(row, 4)?, + )), + seq: Backend::get_value(row, 5)?, + musicbrainz: serde_json::from_str(&Backend::get_value::(row, 6)?)?, + primary_type: serde_json::from_str(&Backend::get_value::(row, 7)?)?, + secondary_types: serde_json::from_str(&Backend::get_value::(row, 8)?)?, + }) + } +} diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index d442002..559c9be 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -203,28 +203,26 @@ impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase #[cfg(test)] mod tests { - // use std::collections::HashMap; + use std::fs; use std::collections::VecDeque; use mockall::{predicate, Sequence}; + use rusqlite::Connection; - use crate::core::{ - // collection::{artist::Artist, Collection}, - testmod::FULL_COLLECTION, - }; + use crate::core::{collection::Collection, testmod::FULL_COLLECTION}; use super::*; - // fn expected() -> Collection { - // let mut expected = FULL_COLLECTION.to_owned(); - // for artist in expected.iter_mut() { - // for album in artist.albums.iter_mut() { - // album.tracks.clear(); - // } - // } - // expected - // } + fn expected() -> Collection { + let mut expected = FULL_COLLECTION.to_owned(); + for artist in expected.iter_mut() { + for album in artist.albums.iter_mut() { + album.tracks.clear(); + } + } + expected + } struct SqlDatabaseTestBackend { pub txs: VecDeque, @@ -323,19 +321,50 @@ mod tests { assert!(database(VecDeque::from([tx])).save(&write_data).is_ok()); } - // #[test] - // fn load() { - // let expected = expected(); - // let result = Ok(DATABASE_JSON.to_owned()); - // eprintln!("{DATABASE_JSON}"); + #[test] + fn load() { + let path = fs::canonicalize("./src/external/database/sql/testmod.db").unwrap(); + let db = Connection::open(path).unwrap(); - // let mut backend = MockISqlDatabaseBackend::new(); - // backend.expect_read().times(1).return_once(|| result); + let mut tx = MockISqlTransactionBackend::new(); + let mut seq = Sequence::new(); - // let read_data: Vec = SqlDatabase::new(backend).load().unwrap(); + let version_query = "SELECT value FROM database_metadata WHERE name = 'version'"; + let version: String = db.query_row(&version_query, (), |row| row.get(0)).unwrap(); + then!(tx, seq, expect_select_database_version).return_once(|| Ok(Some(version))); - // assert_eq!(read_data, expected); - // } + let artist_query = "SELECT name, sort, mbid, properties FROM artists"; + let mut artist_query = db.prepare(&artist_query).unwrap(); + let mut artist_rows = artist_query.query(()).unwrap(); + let mut de_artists = vec![]; + while let Some(row) = artist_rows.next().unwrap() { + de_artists.push(row.try_into().unwrap()); + } + let artists: Collection = de_artists.iter().cloned().map(Into::into).collect(); + then!(tx, seq, expect_select_all_artists).return_once(|| Ok(de_artists)); + + for artist in artists.iter() { + let album_query = + "SELECT title, lib_id, year, month, day, seq, mbid, primary_type, secondary_types + FROM albums WHERE artist_name = ?1"; + let mut album_query = db.prepare(&album_query).unwrap(); + let mut album_rows = album_query.query([artist.meta.id.name.clone()]).unwrap(); + let mut de_albums = vec![]; + while let Some(row) = album_rows.next().unwrap() { + de_albums.push(row.try_into().unwrap()); + } + then!(tx, seq, expect_select_artist_albums) + .with(predicate::eq(artist.meta.id.name.clone())) + .return_once(|_| Ok(de_albums)); + } + + then0!(tx, seq, expect_commit); + + let read_data = database(VecDeque::from([tx])).load().unwrap(); + + let expected = expected(); + assert_eq!(read_data, expected); + } // #[test] // fn reverse() { -- 2.47.1 From 50dd1e373047cbdc381ba2cd2bbdf62225b0097c Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 09:25:59 +0100 Subject: [PATCH 14/20] Load unit test --- src/external/database/serde/common.rs | 6 +- src/external/database/serde/deserialize.rs | 6 +- src/external/database/sql/mod.rs | 65 ++----- src/external/database/sql/testmod.rs | 202 +++++++++++++++++++++ 4 files changed, 222 insertions(+), 57 deletions(-) create mode 100644 src/external/database/sql/testmod.rs diff --git a/src/external/database/serde/common.rs b/src/external/database/serde/common.rs index 8a2207d..000fc8d 100644 --- a/src/external/database/serde/common.rs +++ b/src/external/database/serde/common.rs @@ -14,7 +14,7 @@ pub enum AlbumLibIdDef { } #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] -pub struct SerdeAlbumLibId(#[serde(with = "AlbumLibIdDef")] AlbumLibId); +pub struct SerdeAlbumLibId(#[serde(with = "AlbumLibIdDef")] pub AlbumLibId); impl From for AlbumLibId { fn from(value: SerdeAlbumLibId) -> Self { @@ -70,7 +70,7 @@ pub enum AlbumPrimaryTypeDef { } #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] -pub struct SerdeAlbumPrimaryType(#[serde(with = "AlbumPrimaryTypeDef")] AlbumPrimaryType); +pub struct SerdeAlbumPrimaryType(#[serde(with = "AlbumPrimaryTypeDef")] pub AlbumPrimaryType); impl From for AlbumPrimaryType { fn from(value: SerdeAlbumPrimaryType) -> Self { @@ -102,7 +102,7 @@ pub enum AlbumSecondaryTypeDef { } #[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq)] -pub struct SerdeAlbumSecondaryType(#[serde(with = "AlbumSecondaryTypeDef")] AlbumSecondaryType); +pub struct SerdeAlbumSecondaryType(#[serde(with = "AlbumSecondaryTypeDef")] pub AlbumSecondaryType); impl From for AlbumSecondaryType { fn from(value: SerdeAlbumSecondaryType) -> Self { diff --git a/src/external/database/serde/deserialize.rs b/src/external/database/serde/deserialize.rs index bd1db1a..688dfc7 100644 --- a/src/external/database/serde/deserialize.rs +++ b/src/external/database/serde/deserialize.rs @@ -51,10 +51,12 @@ pub struct DeserializeAlbum { } #[derive(Clone, Debug, Deserialize)] -pub struct DeserializeMbRefOption(#[serde(with = "MbRefOptionDef")] MbRefOption); +pub struct DeserializeMbRefOption( + #[serde(with = "MbRefOptionDef")] pub MbRefOption, +); #[derive(Clone, Debug)] -pub struct DeserializeMbid(Mbid); +pub struct DeserializeMbid(pub Mbid); macro_rules! impl_from_for_mb_ref_option { ($ref:ty) => { diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 559c9be..0cdfeae 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -198,19 +198,21 @@ impl ISqlDatabaseBackend<'conn>> IDatabase for SqlDatabase } } -// #[cfg(test)] -// pub mod testmod; +#[cfg(test)] +pub mod testmod; #[cfg(test)] mod tests { - use std::fs; - use std::collections::VecDeque; use mockall::{predicate, Sequence}; - use rusqlite::Connection; - use crate::core::{collection::Collection, testmod::FULL_COLLECTION}; + use crate::{ + core::{collection::Collection, testmod::FULL_COLLECTION}, + external::database::sql::testmod::{ + DATABASE_SQL_ALBUMS, DATABASE_SQL_ARTISTS, DATABASE_SQL_VERSION, + }, + }; use super::*; @@ -323,39 +325,21 @@ mod tests { #[test] fn load() { - let path = fs::canonicalize("./src/external/database/sql/testmod.db").unwrap(); - let db = Connection::open(path).unwrap(); - let mut tx = MockISqlTransactionBackend::new(); let mut seq = Sequence::new(); - let version_query = "SELECT value FROM database_metadata WHERE name = 'version'"; - let version: String = db.query_row(&version_query, (), |row| row.get(0)).unwrap(); - then!(tx, seq, expect_select_database_version).return_once(|| Ok(Some(version))); + then!(tx, seq, expect_select_database_version) + .return_once(|| Ok(Some(DATABASE_SQL_VERSION.to_string()))); - let artist_query = "SELECT name, sort, mbid, properties FROM artists"; - let mut artist_query = db.prepare(&artist_query).unwrap(); - let mut artist_rows = artist_query.query(()).unwrap(); - let mut de_artists = vec![]; - while let Some(row) = artist_rows.next().unwrap() { - de_artists.push(row.try_into().unwrap()); - } + let de_artists = DATABASE_SQL_ARTISTS.to_owned(); let artists: Collection = de_artists.iter().cloned().map(Into::into).collect(); then!(tx, seq, expect_select_all_artists).return_once(|| Ok(de_artists)); for artist in artists.iter() { - let album_query = - "SELECT title, lib_id, year, month, day, seq, mbid, primary_type, secondary_types - FROM albums WHERE artist_name = ?1"; - let mut album_query = db.prepare(&album_query).unwrap(); - let mut album_rows = album_query.query([artist.meta.id.name.clone()]).unwrap(); - let mut de_albums = vec![]; - while let Some(row) = album_rows.next().unwrap() { - de_albums.push(row.try_into().unwrap()); - } + let de_albums = DATABASE_SQL_ALBUMS.get(&artist.meta.id.name).unwrap(); then!(tx, seq, expect_select_artist_albums) .with(predicate::eq(artist.meta.id.name.clone())) - .return_once(|_| Ok(de_albums)); + .return_once(|_| Ok(de_albums.to_owned())); } then0!(tx, seq, expect_commit); @@ -366,29 +350,6 @@ mod tests { assert_eq!(read_data, expected); } - // #[test] - // fn reverse() { - // let input = DATABASE_JSON.to_owned(); - // let result = Ok(input.clone()); - - // let mut backend = MockISqlDatabaseBackend::new(); - // backend - // .expect_write() - // .with(predicate::eq(input)) - // .times(1) - // .return_once(|_| Ok(())); - // backend.expect_read().times(1).return_once(|| result); - // let mut database = SqlDatabase::new(backend); - - // let write_data = FULL_COLLECTION.to_owned(); - // database.save(&write_data).unwrap(); - // let read_data: Vec = database.load().unwrap(); - - // // Album information is not saved to disk. - // let expected = expected(); - // assert_eq!(read_data, expected); - // } - // #[test] // fn load_errors() { // let json = String::from(""); diff --git a/src/external/database/sql/testmod.rs b/src/external/database/sql/testmod.rs new file mode 100644 index 0000000..c03a880 --- /dev/null +++ b/src/external/database/sql/testmod.rs @@ -0,0 +1,202 @@ +use std::collections::HashMap; + +use once_cell::sync::Lazy; + +use crate::{ + collection::{ + album::{AlbumDate, AlbumLibId, AlbumPrimaryType}, + musicbrainz::MbRefOption, + }, + external::database::serde::{ + common::{SerdeAlbumDate, SerdeAlbumLibId, SerdeAlbumPrimaryType}, + deserialize::{ + DeserializeAlbum, DeserializeArtist, DeserializeMbRefOption, DeserializeMbid, + }, + }, +}; + +pub static DATABASE_SQL_VERSION: &str = "V20250103"; + +pub static DATABASE_SQL_ARTISTS: Lazy> = Lazy::new(|| { + vec![ + DeserializeArtist { + name: String::from("Album_Artist ‘A’"), + sort: None, + musicbrainz: DeserializeMbRefOption(MbRefOption::Some(DeserializeMbid( + "00000000-0000-0000-0000-000000000000".try_into().unwrap(), + ))), + properties: HashMap::from([ + ( + String::from("MusicButler"), + vec![String::from( + "https://www.musicbutler.io/artist-page/000000000", + )], + ), + ( + String::from("Qobuz"), + vec![String::from( + "https://www.qobuz.com/nl-nl/interpreter/artist-a/download-streaming-albums", + )], + ), + ]), + albums: vec![], + }, + DeserializeArtist { + name: String::from("Album_Artist ‘B’"), + sort: None, + musicbrainz: DeserializeMbRefOption(MbRefOption::Some(DeserializeMbid( + "11111111-1111-1111-1111-111111111111".try_into().unwrap(), + ))), + properties: HashMap::from([ + (String::from("MusicButler"), vec![ + String::from("https://www.musicbutler.io/artist-page/111111111"), + String::from("https://www.musicbutler.io/artist-page/111111112"), + ]), + (String::from("Bandcamp"), vec![ + String::from("https://artist-b.bandcamp.com/") + ]), + (String::from("Qobuz"), vec![ + String::from( + "https://www.qobuz.com/nl-nl/interpreter/artist-b/download-streaming-albums", + ) + ]), + ]), + albums: vec![], + }, + DeserializeArtist { + name: String::from("The Album_Artist ‘C’"), + sort: Some(String::from("Album_Artist ‘C’, The")), + musicbrainz: DeserializeMbRefOption(MbRefOption::CannotHaveMbid), + properties: HashMap::new(), + albums: vec![], + }, + DeserializeArtist { + name: String::from("Album_Artist ‘D’"), + sort: None, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + properties: HashMap::new(), + albums: vec![], + }, + ] +}); + +pub static DATABASE_SQL_ALBUMS: Lazy>> = Lazy::new(|| { + HashMap::from([ + ( + String::from("Album_Artist ‘A’"), + vec![ + DeserializeAlbum { + title: String::from("album_title a.a"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(1)), + date: SerdeAlbumDate(AlbumDate::new(Some(1998), None, None)), + seq: 1, + musicbrainz: DeserializeMbRefOption(MbRefOption::Some(DeserializeMbid( + "00000000-0000-0000-0000-000000000000".try_into().unwrap(), + ))), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + DeserializeAlbum { + title: String::from("album_title a.b"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(2)), + date: SerdeAlbumDate(AlbumDate::new(Some(2015), Some(4), None)), + seq: 1, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + ], + ), + ( + String::from("Album_Artist ‘B’"), + vec![ + DeserializeAlbum { + title: String::from("album_title b.a"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(3)), + date: SerdeAlbumDate(AlbumDate::new(Some(2003), Some(6), Some(6))), + seq: 1, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + DeserializeAlbum { + title: String::from("album_title b.b"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(4)), + date: SerdeAlbumDate(AlbumDate::new(Some(2008), None, None)), + seq: 3, + musicbrainz: DeserializeMbRefOption(MbRefOption::Some(DeserializeMbid( + "11111111-1111-1111-1111-111111111111".try_into().unwrap(), + ))), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + DeserializeAlbum { + title: String::from("album_title b.c"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(5)), + date: SerdeAlbumDate(AlbumDate::new(Some(2009), None, None)), + seq: 2, + musicbrainz: DeserializeMbRefOption(MbRefOption::Some(DeserializeMbid( + "11111111-1111-1111-1111-111111111112".try_into().unwrap(), + ))), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + DeserializeAlbum { + title: String::from("album_title b.d"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(6)), + date: SerdeAlbumDate(AlbumDate::new(Some(2015), None, None)), + seq: 4, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + ], + ), + ( + String::from("The Album_Artist ‘C’"), + vec![ + DeserializeAlbum { + title: String::from("album_title c.a"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(7)), + date: SerdeAlbumDate(AlbumDate::new(Some(1985), None, None)), + seq: 0, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + DeserializeAlbum { + title: String::from("album_title c.b"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(8)), + date: SerdeAlbumDate(AlbumDate::new(Some(2018), None, None)), + seq: 0, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + ], + ), + ( + String::from("Album_Artist ‘D’"), + vec![ + DeserializeAlbum { + title: String::from("album_title d.a"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(9)), + date: SerdeAlbumDate(AlbumDate::new(Some(1995), None, None)), + seq: 0, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + DeserializeAlbum { + title: String::from("album_title d.b"), + lib_id: SerdeAlbumLibId(AlbumLibId::Value(10)), + date: SerdeAlbumDate(AlbumDate::new(Some(2028), None, None)), + seq: 0, + musicbrainz: DeserializeMbRefOption(MbRefOption::None), + primary_type: Some(SerdeAlbumPrimaryType(AlbumPrimaryType::Album)), + secondary_types: vec![], + }, + ], + ), + ]) +}); -- 2.47.1 From 8c55b79777f38249f5a667061db1f44f92a5efd8 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 09:27:13 +0100 Subject: [PATCH 15/20] Move definition --- src/external/database/sql/backend.rs | 6 ------ src/external/database/sql/mod.rs | 6 ++++++ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index 9051ae9..9c7de69 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -71,12 +71,6 @@ impl SqlTransactionSqliteBackend<'_> { } } -impl From for Error { - fn from(value: serde_json::Error) -> Self { - Error::SerDeError(value.to_string()) - } -} - impl<'conn> ISqlDatabaseBackend<'conn> for SqlDatabaseSqliteBackend { type Tx = SqlTransactionSqliteBackend<'conn>; diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 0cdfeae..697cd63 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -86,6 +86,12 @@ pub enum Error { ExecError(String), } +impl From for Error { + fn from(value: serde_json::Error) -> Self { + Error::SerDeError(value.to_string()) + } +} + impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { -- 2.47.1 From 701719834e1aa5fd727058a2084db24f1df97569 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 10:02:52 +0100 Subject: [PATCH 16/20] Complete UT coverage --- src/external/database/sql/backend.rs | 38 ++++---- src/external/database/sql/mod.rs | 124 ++++++++++++++++++++------- 2 files changed, 112 insertions(+), 50 deletions(-) diff --git a/src/external/database/sql/backend.rs b/src/external/database/sql/backend.rs index 9c7de69..3bc9cb6 100644 --- a/src/external/database/sql/backend.rs +++ b/src/external/database/sql/backend.rs @@ -37,16 +37,14 @@ impl SqlDatabaseSqliteBackend { } impl SqlTransactionSqliteBackend<'_> { - fn prepare(&self, sql: &str) -> Result { - self.tx - .prepare(sql) - .map_err(|err| Error::StmtError(err.to_string())) + // We only prepare strings known at compile time so errors in prep are bugs. + fn prepare(&self, sql: &'static str) -> Statement { + self.tx.prepare(sql).unwrap() } - fn prepare_cached(&self, sql: &str) -> Result { - self.tx - .prepare_cached(sql) - .map_err(|err| Error::StmtError(err.to_string())) + // We only prepare strings known at compile time so errors in prep are bugs. + fn prepare_cached(&self, sql: &'static str) -> CachedStatement { + self.tx.prepare_cached(sql).unwrap() } fn execute(stmt: &mut Statement, params: P) -> Result<(), Error> { @@ -95,12 +93,12 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { name TEXT NOT NULL PRIMARY KEY, value TEXT NOT NULL )", - )?; + ); Self::execute(&mut stmt, ()) } fn drop_database_metadata_table(&self) -> Result<(), Error> { - let mut stmt = self.prepare_cached("DROP TABLE database_metadata")?; + let mut stmt = self.prepare_cached("DROP TABLE database_metadata"); Self::execute(&mut stmt, ()) } @@ -112,12 +110,12 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { mbid JSON NOT NULL DEFAULT '\"None\"', properties JSON NOT NULL DEFAULT '{}' )", - )?; + ); Self::execute(&mut stmt, ()) } fn drop_artists_table(&self) -> Result<(), Error> { - let mut stmt = self.prepare_cached("DROP TABLE artists")?; + let mut stmt = self.prepare_cached("DROP TABLE artists"); Self::execute(&mut stmt, ()) } @@ -136,12 +134,12 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { secondary_types JSON NOT NULL DEFAULT '[]', FOREIGN KEY (artist_name) REFERENCES artists(name) ON DELETE CASCADE ON UPDATE NO ACTION )", - )?; + ); Self::execute(&mut stmt, ()) } fn drop_albums_table(&self) -> Result<(), Error> { - let mut stmt = self.prepare_cached("DROP TABLE albums")?; + let mut stmt = self.prepare_cached("DROP TABLE albums"); Self::execute(&mut stmt, ()) } @@ -149,13 +147,13 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { let mut stmt = self.prepare_cached( "INSERT INTO database_metadata (name, value) VALUES (?1, ?2)", - )?; + ); Self::execute(&mut stmt, ("version", version)) } fn select_database_version<'a>(&self) -> Result, Error> { let mut stmt = - self.prepare_cached("SELECT value FROM database_metadata WHERE name = 'version'")?; + self.prepare_cached("SELECT value FROM database_metadata WHERE name = 'version'"); let mut rows = Self::query(&mut stmt, ())?; Self::next_row(&mut rows)? @@ -167,7 +165,7 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { let mut stmt = self.prepare_cached( "INSERT INTO artists (name, sort, mbid, properties) VALUES (?1, ?2, ?3, ?4)", - )?; + ); Self::execute( &mut stmt, ( @@ -180,7 +178,7 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { } fn select_all_artists(&self) -> Result, Error> { - let mut stmt = self.prepare_cached("SELECT name, sort, mbid, properties FROM artists")?; + let mut stmt = self.prepare_cached("SELECT name, sort, mbid, properties FROM artists"); let mut rows = Self::query(&mut stmt, ())?; let mut artists = vec![]; @@ -196,7 +194,7 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { "INSERT INTO albums (title, lib_id, mbid, artist_name, year, month, day, seq, primary_type, secondary_types) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10)", - )?; + ); Self::execute( &mut stmt, ( @@ -218,7 +216,7 @@ impl ISqlTransactionBackend for SqlTransactionSqliteBackend<'_> { let mut stmt = self.prepare_cached( "SELECT title, lib_id, year, month, day, seq, mbid, primary_type, secondary_types FROM albums WHERE artist_name = ?1", - )?; + ); let mut rows = Self::query(&mut stmt, [artist_name])?; let mut albums = vec![]; diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 697cd63..85decb7 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -78,11 +78,9 @@ pub trait ISqlTransactionBackend { pub enum Error { /// An error occurred when connecting to the database. OpenError(String), - /// An error occurred when preparing a statement for execution. - StmtError(String), /// An error occurred during serialisation. SerDeError(String), - /// An error occurred during execution. + /// An error occurred during SQL execution. ExecError(String), } @@ -98,12 +96,8 @@ impl fmt::Display for Error { Self::OpenError(ref s) => { write!(f, "an error occurred when connecting to the database: {s}") } - Self::StmtError(ref s) => write!( - f, - "an error occurred when preparing a statement for execution: {s}" - ), Self::SerDeError(ref s) => write!(f, "an error occurred during serialisation : {s}"), - Self::ExecError(ref s) => write!(f, "an error occurred during execution: {s}"), + Self::ExecError(ref s) => write!(f, "an error occurred during SQL execution: {s}"), } } } @@ -246,7 +240,9 @@ mod tests { type Tx = MockISqlTransactionBackend; fn transaction(&mut self) -> Result { - Ok(self.txs.pop_front().unwrap()) + self.txs + .pop_front() + .ok_or_else(|| Error::OpenError(String::from("lol"))) } } @@ -309,7 +305,6 @@ mod tests { let write_data = FULL_COLLECTION.to_owned(); let mut tx = MockISqlTransactionBackend::new(); - let mut seq = Sequence::new(); expect_drop!(tx, seq); expect_create!(tx, seq); @@ -356,27 +351,96 @@ mod tests { assert_eq!(read_data, expected); } - // #[test] - // fn load_errors() { - // let json = String::from(""); - // let serde_err = serde_json::from_str::(&json); - // assert!(serde_err.is_err()); + #[test] + fn load_missing_database_version() { + let mut tx = MockISqlTransactionBackend::new(); + let mut seq = Sequence::new(); + then!(tx, seq, expect_select_database_version).return_once(|| Ok(None)); + let error = database(VecDeque::from([tx])).load().unwrap_err(); + assert!(matches!(error, LoadError::SerDeError(_))); + } - // let serde_err: LoadError = serde_err.unwrap_err().into(); - // assert!(!serde_err.to_string().is_empty()); - // assert!(!format!("{:?}", serde_err).is_empty()); - // } + #[test] + fn load_unknown_database_version() { + let mut tx = MockISqlTransactionBackend::new(); + let mut seq = Sequence::new(); + then!(tx, seq, expect_select_database_version) + .return_once(|| Ok(Some(String::from("no u")))); + let error = database(VecDeque::from([tx])).load().unwrap_err(); + assert!(matches!(error, LoadError::SerDeError(_))); + } - // #[test] - // fn save_errors() { - // // serde_json will raise an error as it has certain requirements on keys. - // let mut object = HashMap::, String>::new(); - // object.insert(Ok(()), String::from("string")); - // let serde_err = serde_json::to_string(&object); - // assert!(serde_err.is_err()); + #[test] + fn load_backend_open_error() { + let error = database(VecDeque::from([])).load().unwrap_err(); + assert!(matches!(error, LoadError::IoError(_))); + } - // let serde_err: SaveError = serde_err.unwrap_err().into(); - // assert!(!serde_err.to_string().is_empty()); - // assert!(!format!("{:?}", serde_err).is_empty()); - // } + #[test] + fn save_backend_open_error() { + let error = database(VecDeque::from([])).save(&vec![]).unwrap_err(); + assert!(matches!(error, SaveError::IoError(_))); + } + + #[test] + fn load_backend_exec_error() { + let mut tx = MockISqlTransactionBackend::new(); + let mut seq = Sequence::new(); + then!(tx, seq, expect_select_database_version) + .return_once(|| Err(Error::ExecError(String::from("serde")))); + let error = database(VecDeque::from([tx])).load().unwrap_err(); + assert!(matches!(error, LoadError::IoError(_))); + } + + #[test] + fn save_backend_exec_error() { + let mut tx = MockISqlTransactionBackend::new(); + let mut seq = Sequence::new(); + expect_drop!(tx, seq); + expect_create!(tx, seq); + then!(tx, seq, expect_insert_database_version) + .with(predicate::eq(V20250103)) + .return_once(|_| Err(Error::ExecError(String::from("exec")))); + + let error = database(VecDeque::from([tx])).save(&vec![]).unwrap_err(); + assert!(matches!(error, SaveError::IoError(_))); + } + + #[test] + fn load_backend_serde_error() { + let mut tx = MockISqlTransactionBackend::new(); + let mut seq = Sequence::new(); + + then!(tx, seq, expect_select_database_version) + .return_once(|| Ok(Some(DATABASE_SQL_VERSION.to_string()))); + then!(tx, seq, expect_select_all_artists) + .return_once(|| Err(Error::SerDeError(String::from("serde")))); + + let error = database(VecDeque::from([tx])).load().unwrap_err(); + assert!(matches!(error, LoadError::SerDeError(_))); + } + + #[test] + fn save_backend_serde_error() { + let write_data = FULL_COLLECTION.to_owned(); + + let mut tx = MockISqlTransactionBackend::new(); + let mut seq = Sequence::new(); + expect_drop!(tx, seq); + expect_create!(tx, seq); + then1!(tx, seq, expect_insert_database_version).with(predicate::eq(V20250103)); + then!(tx, seq, expect_insert_artist) + .return_once(|_| Err(Error::SerDeError(String::from("serde")))); + + let error = database(VecDeque::from([tx])).save(&write_data).unwrap_err(); + assert!(matches!(error, SaveError::SerDeError(_))); + } + + #[test] + fn serde_json_error() { + let error = serde_json::from_str::("").unwrap_err(); + let error: Error = error.into(); + assert!(matches!(error, Error::SerDeError(_))); + assert!(!error.to_string().is_empty()); + } } -- 2.47.1 From 21ba413df4760c1e7deec60ff533be6a07514bc2 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 10:03:16 +0100 Subject: [PATCH 17/20] Lint --- src/external/database/sql/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/external/database/sql/mod.rs b/src/external/database/sql/mod.rs index 85decb7..78a3305 100644 --- a/src/external/database/sql/mod.rs +++ b/src/external/database/sql/mod.rs @@ -432,7 +432,9 @@ mod tests { then!(tx, seq, expect_insert_artist) .return_once(|_| Err(Error::SerDeError(String::from("serde")))); - let error = database(VecDeque::from([tx])).save(&write_data).unwrap_err(); + let error = database(VecDeque::from([tx])) + .save(&write_data) + .unwrap_err(); assert!(matches!(error, SaveError::SerDeError(_))); } -- 2.47.1 From 0695740666fa2f17c8b4ee94868f200b9ea32559 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 10:06:03 +0100 Subject: [PATCH 18/20] Ensure sorting when deserializing --- src/external/database/serde/deserialize.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/external/database/serde/deserialize.rs b/src/external/database/serde/deserialize.rs index 688dfc7..007099e 100644 --- a/src/external/database/serde/deserialize.rs +++ b/src/external/database/serde/deserialize.rs @@ -23,8 +23,10 @@ pub enum DeserializeDatabase { impl From for Collection { fn from(database: DeserializeDatabase) -> Self { match database { - DeserializeDatabase::V20250103(collection) => { - collection.into_iter().map(Into::into).collect() + DeserializeDatabase::V20250103(db) => { + let mut collection: Collection = db.into_iter().map(Into::into).collect(); + collection.sort_unstable(); + collection } } } -- 2.47.1 From b6b25f71985ca609c7de04e9abbedecf86cb8193 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 10:20:34 +0100 Subject: [PATCH 19/20] Re-export bundled feature of rusqlite --- Cargo.lock | 1 + Cargo.toml | 1 + 2 files changed, 2 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index a260e3e..9e66236 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -844,6 +844,7 @@ version = "0.30.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2e99fb7a497b1e3339bc746195567ed8d3e24945ecd636e3619d20b9de9e9149" dependencies = [ + "cc", "pkg-config", "vcpkg", ] diff --git a/Cargo.toml b/Cargo.toml index a47e9c6..a1df039 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ tempfile = "3.15.0" default = ["database-json", "library-beets"] bin = ["structopt"] database-sqlite = ["rusqlite", "serde", "serde_json"] +database-sqlite-bundled = ["rusqlite/bundled"] database-json = ["serde", "serde_json"] library-beets = [] library-beets-ssh = ["openssh", "tokio"] -- 2.47.1 From dce2756e7ef3438b1ec2e4150e9b7456c68a6b55 Mon Sep 17 00:00:00 2001 From: Wojciech Kozlowski Date: Sun, 12 Jan 2025 10:22:22 +0100 Subject: [PATCH 20/20] Update README --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 7e1bc40..fc478b0 100644 --- a/README.md +++ b/README.md @@ -4,6 +4,19 @@ ### Pre-requisites +#### database-sqlite + +This feature requires the `sqlite` library. + +Either install system libraries: with + +On Fedora: +``` sh +sudo dnf install sqlite-devel +``` + +Or use a bundled version by enabling the `database-sqlite-bundled` feature. + #### musicbrainz-api This feature requires the `openssl` system library. -- 2.47.1