From 094d9fed1e44ab943cf46df596e0663c04b80282 Mon Sep 17 00:00:00 2001 From: Polochon_street Date: Tue, 30 Jul 2024 19:13:10 +0200 Subject: [PATCH] Add a mechanism to perform database migrations --- CHANGELOG.md | 5 + Cargo.lock | 2 +- Cargo.toml | 2 +- data/old_database.sql | 135 +++++++++++++++++++++ src/library.rs | 274 +++++++++++++++++++++++++++++++++--------- 5 files changed, 361 insertions(+), 57 deletions(-) create mode 100644 data/old_database.sql diff --git a/CHANGELOG.md b/CHANGELOG.md index 87c4c8d..126aeae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## bliss 0.9.0 +* Add a mechanism to do migrations for Libraries, to make sure we're ready + for potential new features. +* Make `track_number` an integer, and not a string. + ## bliss 0.8.0 * Remove the `number_songs` option from `Library::playlist_from_custom`. Since it now returns an Iterator, `number_songs` can just be replaced by `take`. diff --git a/Cargo.lock b/Cargo.lock index e1822e0..e18a754 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -138,7 +138,7 @@ checksum = "b048fb63fd8b5923fc5aa7b340d8e156aec7ec02f0c78fa8a6ddc2613f6f71de" [[package]] name = "bliss-audio" -version = "0.8.0" +version = "0.9.0" dependencies = [ "adler32", "anyhow", diff --git a/Cargo.toml b/Cargo.toml index dbc9731..4f64401 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "bliss-audio" -version = "0.8.0" +version = "0.9.0" build = "build.rs" authors = ["Polochon-street "] edition = "2021" diff --git a/data/old_database.sql b/data/old_database.sql new file mode 100644 index 0000000..5a3e7eb --- /dev/null +++ b/data/old_database.sql @@ -0,0 +1,135 @@ +create table song ( + id integer primary key, + path text not null unique, + duration float, + album_artist text, + artist text, + title text, + album text, + track_number text, + genre text, + cue_path text, + audio_file_path text, + stamp timestamp default current_timestamp, + version integer, + analyzed boolean default false, + extra_info json, + error text +); +create table feature ( + id integer primary key, + song_id integer not null, + feature real not null, + feature_index integer not null, + unique(song_id, feature_index), + foreign key(song_id) references song(id) on delete cascade +); + +insert into song ( + id, path, artist, title, album, album_artist, + track_number, genre, stamp, version, duration, analyzed, + extra_info +) +values ( + 1, '/random/path', 'Some Artist', 'A Title', 'Some Album', + 'Some Album Artist', '01', 'Electronica', '2022-01-01', + 1, 250, true, '{\"key\": \"value\"}' +), +( + 2, '/random/path2', 'Some Artist', 'A Title', 'Some Album', + 'Some Album Artist', '', 'Electronica', '2022-01-01', + 1, 250, true, '{\"key\": \"value\"}' +), +( + 3, '/random/path3', 'Some Artist', 'A Title', 'Some Album', + 'Some Album Artist', null, 'Electronica', '2022-01-01', + 1, 250, true, '{\"key\": \"value\"}' +), +( + 4, '/random/path4', 'Some Artist', 'A Title', 'Some Album', + 'Some Album Artist', 'test', 'Electronica', '2022-01-01', + 1, 250, true, '{\"key\": \"value\"}' +); + +insert into feature (song_id, feature, feature_index) +values + (1, 1.1, 1), + (1, 1.1, 2), + (1, 1.1, 3), + (1, 1.1, 4), + (1, 1.1, 5), + (1, 1.1, 6), + (1, 1.1, 7), + (1, 1.1, 8), + (1, 1.1, 9), + (1, 1.1, 10), + (1, 1.1, 11), + (1, 1.1, 12), + (1, 1.1, 13), + (1, 1.1, 14), + (1, 1.1, 15), + (1, 1.1, 16), + (1, 1.1, 17), + (1, 1.1, 18), + (1, 1.1, 19), + (1, 1.1, 20), + (2, 2.1, 1), + (2, 2.1, 2), + (2, 2.1, 3), + (2, 2.1, 4), + (2, 2.1, 5), + (2, 2.1, 6), + (2, 2.1, 7), + (2, 2.1, 8), + (2, 2.1, 9), + (2, 2.1, 10), + (2, 2.1, 11), + (2, 2.1, 12), + (2, 2.1, 13), + (2, 2.1, 14), + (2, 2.1, 15), + (2, 2.1, 16), + (2, 2.1, 17), + (2, 2.1, 18), + (2, 2.1, 19), + (2, 2.1, 20), + (3, 3.1, 1), + (3, 3.1, 2), + (3, 3.1, 3), + (3, 3.1, 4), + (3, 3.1, 5), + (3, 3.1, 6), + (3, 3.1, 7), + (3, 3.1, 8), + (3, 3.1, 9), + (3, 3.1, 10), + (3, 3.1, 11), + (3, 3.1, 12), + (3, 3.1, 13), + (3, 3.1, 14), + (3, 3.1, 15), + (3, 3.1, 16), + (3, 3.1, 17), + (3, 3.1, 18), + (3, 3.1, 19), + (3, 3.1, 20), + (4, 4.1, 1), + (4, 4.1, 2), + (4, 4.1, 3), + (4, 4.1, 4), + (4, 4.1, 5), + (4, 4.1, 6), + (4, 4.1, 7), + (4, 4.1, 8), + (4, 4.1, 9), + (4, 4.1, 10), + (4, 4.1, 11), + (4, 4.1, 12), + (4, 4.1, 13), + (4, 4.1, 14), + (4, 4.1, 15), + (4, 4.1, 16), + (4, 4.1, 17), + (4, 4.1, 18), + (4, 4.1, 19), + (4, 4.1, 20); diff --git a/src/library.rs b/src/library.rs index 324ba5b..7302c94 100644 --- a/src/library.rs +++ b/src/library.rs @@ -354,17 +354,52 @@ impl AsRef for LibrarySong { } // TODO add logging statement -// TODO concrete examples -// TODO example LibrarySong without any extra_info // TODO maybe return number of elements updated / deleted / whatev in analysis // functions? -// TODO add full rescan -// TODO a song_from_path with custom filters -// TODO "smart" playlist // TODO should it really use anyhow errors? // TODO make sure that the path to string is consistent -// TODO make a function that returns a list of all analyzed songs in the db impl Library { + const SQLITE_SCHEMA: &'static str = " + create table song ( + id integer primary key, + path text not null unique, + duration float, + album_artist text, + artist text, + title text, + album text, + track_number integer, + genre text, + cue_path text, + audio_file_path text, + stamp timestamp default current_timestamp, + version integer, + analyzed boolean default false, + extra_info json, + error text + ); + pragma foreign_keys = on; + create table feature ( + id integer primary key, + song_id integer not null, + feature real not null, + feature_index integer not null, + unique(song_id, feature_index), + foreign key(song_id) references song(id) on delete cascade + ) + "; + const SQLITE_MIGRATIONS: &'static [&'static str] = &[ + "", + " + alter table song add column track_number_1 integer; + update song set track_number_1 = s1.cast_track_number from ( + select cast(track_number as int) as cast_track_number, id from song + ) as s1 where s1.id = song.id and cast(track_number as int) != 0; + alter table song drop column track_number; + alter table song rename column track_number_1 to track_number; + ", + ]; + /// Create a new [Library] object from the given Config struct that /// implements the [AppConfigTrait]. /// writing the configuration to the file given in @@ -390,43 +425,11 @@ impl Library { create_dir_all(config.base_config().config_path.parent().unwrap())?; } let sqlite_conn = Connection::open(&config.base_config().database_path)?; - sqlite_conn.execute( - " - create table if not exists song ( - id integer primary key, - path text not null unique, - duration float, - album_artist text, - artist text, - title text, - album text, - track_number integer, - genre text, - cue_path text, - audio_file_path text, - stamp timestamp default current_timestamp, - version integer, - analyzed boolean default false, - extra_info json, - error text - ); - ", - [], - )?; - sqlite_conn.execute("pragma foreign_keys = on;", [])?; - sqlite_conn.execute( - " - create table if not exists feature ( - id integer primary key, - song_id integer not null, - feature real not null, - feature_index integer not null, - unique(song_id, feature_index), - foreign key(song_id) references song(id) on delete cascade - ) - ", - [], - )?; + + Library::::upgrade(&sqlite_conn).map_err(|e| { + BlissError::ProviderError(format!("Could not run database upgrade: {}", e)) + })?; + config.write()?; Ok(Self { config, @@ -435,6 +438,59 @@ impl Library { }) } + fn upgrade(sqlite_conn: &Connection) -> Result<()> { + let version: u32 = sqlite_conn + .query_row("pragma user_version", [], |row| row.get(0)) + .map_err(|e| { + BlissError::ProviderError(format!("Could not get database version: {}.", e)) + })?; + + let migrations = Library::::SQLITE_MIGRATIONS; + match version.cmp(&(migrations.len() as u32)) { + std::cmp::Ordering::Equal => return Ok(()), + std::cmp::Ordering::Greater => bail!(format!( + "bliss-rs version {} is older than the schema version {}", + version, + migrations.len() + )), + _ => (), + }; + + let number_tables: u32 = sqlite_conn + .query_row("select count(*) from pragma_table_list", [], |row| { + row.get(0) + }) + .map_err(|e| { + BlissError::ProviderError(format!( + "Could not query initial database information: {}", + e + )) + })?; + let is_database_new = number_tables <= 2; + + if version == 0 && is_database_new { + sqlite_conn + .execute_batch(Library::::SQLITE_SCHEMA) + .map_err(|e| { + BlissError::ProviderError(format!("Could not initialize schema: {}.", e)) + })?; + } else { + for migration in migrations.iter().skip(version as usize) { + sqlite_conn.execute_batch(migration).map_err(|e| { + BlissError::ProviderError(format!("Could not execute migration: {}.", e)) + })?; + } + } + + sqlite_conn + .execute(&format!("pragma user_version = {}", migrations.len()), []) + .map_err(|e| { + BlissError::ProviderError(format!("Could not update database version: {}.", e)) + })?; + + Ok(()) + } + /// Load a library from a configuration path. /// /// If no configuration path is provided, the path is @@ -1674,19 +1730,19 @@ mod test { cue_path, audio_file_path ) values ( 1001, '/path/to/song1001', 'Artist1001', 'Title1001', 'An Album1001', - 'An Album Artist1001', '03', 'Electronica1001', 310, true, + 'An Album Artist1001', 3, 'Electronica1001', 310, true, 1, '{\"ignore\": true, \"metadata_bliss_does_not_have\": \"/path/to/charlie1001\"}', null, null ), ( 2001, '/path/to/song2001', 'Artist2001', 'Title2001', 'An Album2001', - 'An Album Artist2001', '02', 'Electronica2001', 410, true, + 'An Album Artist2001', 2, 'Electronica2001', 410, true, 1, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie2001\"}', null, null ), ( 2201, '/path/to/song2201', 'Artist2001', 'Title2001', 'Remixes of Album2001', - 'An Album Artist2001', '02', 'Electronica2001', 410, true, + 'An Album Artist2001', 2, 'Electronica2001', 410, true, 1, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie2201\"}', null, null ), @@ -1696,32 +1752,32 @@ mod test { ), ( 4001, '/path/to/song4001', 'Artist4001', 'Title4001', 'An Album4001', - 'An Album Artist4001', '01', 'Electronica4001', 510, true, + 'An Album Artist4001', 1, 'Electronica4001', 510, true, 0, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie4001\"}', null, null ), ( 5001, '/path/to/song5001', 'Artist5001', 'Title5001', 'An Album1001', - 'An Album Artist5001', '01', 'Electronica5001', 610, true, + 'An Album Artist5001', 1, 'Electronica5001', 610, true, 1, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie5001\"}', null, null ), ( 6001, '/path/to/song6001', 'Artist6001', 'Title6001', 'An Album2001', - 'An Album Artist6001', '01', 'Electronica6001', 710, true, + 'An Album Artist6001', 1, 'Electronica6001', 710, true, 1, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie6001\"}', null, null ), ( 7001, '/path/to/song7001', 'Artist7001', 'Title7001', 'An Album7001', - 'An Album Artist7001', '01', 'Electronica7001', 810, true, + 'An Album Artist7001', 1, 'Electronica7001', 810, true, 1, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie7001\"}', null, null ), ( 7002, '/path/to/cuetrack.cue/CUE_TRACK001', 'CUE Artist', 'CUE Title 01', 'CUE Album', - 'CUE Album Artist', '01', null, 810, true, + 'CUE Album Artist', 1, null, 810, true, 1, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie7001\"}', '/path/to/cuetrack.cue', '/path/to/cuetrack.flac' @@ -1729,20 +1785,20 @@ mod test { ( 7003, '/path/to/cuetrack.cue/CUE_TRACK002', 'CUE Artist', 'CUE Title 02', 'CUE Album', - 'CUE Album Artist', '02', null, 910, true, + 'CUE Album Artist', 2, null, 910, true, 1, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie7001\"}', '/path/to/cuetrack.cue', '/path/to/cuetrack.flac' ), ( 8001, '/path/to/song8001', 'Artist8001', 'Title8001', 'An Album1001', - 'An Album Artist8001', '03', 'Electronica8001', 910, true, + 'An Album Artist8001', 3, 'Electronica8001', 910, true, 0, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie8001\"}', null, null ), ( 9001, './data/s16_stereo_22_5kHz.flac', 'Artist9001', 'Title9001', - 'An Album9001', 'An Album Artist8001', '03', 'Electronica8001', + 'An Album9001', 'An Album Artist8001', 3, 'Electronica8001', 1010, true, 0, '{\"ignore\": false, \"metadata_bliss_does_not_have\": \"/path/to/charlie7001\"}', null, null ); @@ -1932,6 +1988,9 @@ mod test { .map(|x| x.unwrap()) .collect::>() .try_into() + .map_err(|v| { + BlissError::ProviderError(format!("Could not retrieve analysis for song {} that was supposed to be analyzed: {:?}.", song_path, v)) + }) .unwrap(), }; expected_song.analysis = expected_analysis_vector; @@ -3202,7 +3261,7 @@ mod test { ) values ( 1, '/random/path', 'Some Artist', 'A Title', 'Some Album', - 'Some Album Artist', '01', 'Electronica', '2022-01-01', + 'Some Album Artist', 1, 'Electronica', '2022-01-01', 1, 250, true, '{\"key\": \"value\"}' ); ", @@ -3221,6 +3280,111 @@ mod test { .unwrap(); } + #[test] + #[cfg(feature = "ffmpeg")] + fn test_library_new_database_upgrade() { + let config_dir = TempDir::new("tmp").unwrap(); + let sqlite_db_path = config_dir.path().join("test.db"); + // Initialize the database with the contents of old_database.sq, without + // having anything to do with Library (yet) + { + let sqlite_conn = Connection::open(sqlite_db_path.clone()).unwrap(); + let sql_statements = fs::read_to_string("data/old_database.sql").unwrap(); + sqlite_conn.execute_batch(&sql_statements).unwrap(); + let track_number: String = sqlite_conn + .query_row("select track_number from song where id = 1", [], |row| { + row.get(0) + }) + .unwrap(); + // Check that songs are indeed inserted the old way + assert_eq!(track_number, "01"); + let version: u32 = sqlite_conn + .query_row("pragma user_version", [], |row| row.get(0)) + .unwrap(); + assert_eq!(version, 0); + } + + let library = Library::::new_from_base( + Some(config_dir.path().join("config.txt")), + Some(sqlite_db_path), + NonZeroUsize::new(1), + ) + .unwrap(); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + Some(1), + _basic_song_from_database(sqlite_conn, "/random/path").track_number + ); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + None, + _basic_song_from_database(sqlite_conn, "/random/path2").track_number + ); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + None, + _basic_song_from_database(sqlite_conn, "/random/path3").track_number + ); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + None, + _basic_song_from_database(sqlite_conn, "/random/path4").track_number + ); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + let version: u32 = sqlite_conn + .query_row("pragma user_version", [], |row| row.get(0)) + .unwrap(); + assert_eq!(version, 2); + } + + #[test] + #[cfg(feature = "ffmpeg")] + fn test_library_new_database_already_last_version() { + let config_dir = TempDir::new("tmp").unwrap(); + let sqlite_db_path = config_dir.path().join("test.db"); + // Initialize the database with the contents of old_database.sq, without + // having anything to do with Library (yet) + { + let sqlite_conn = Connection::open(sqlite_db_path.clone()).unwrap(); + let sql_statements = fs::read_to_string("data/old_database.sql").unwrap(); + sqlite_conn.execute_batch(&sql_statements).unwrap(); + let track_number: String = sqlite_conn + .query_row("select track_number from song where id = 1", [], |row| { + row.get(0) + }) + .unwrap(); + // Check that songs are indeed inserted the old way + assert_eq!(track_number, "01"); + } + + let library = Library::::new_from_base( + Some(config_dir.path().join("config.txt")), + Some(sqlite_db_path), + NonZeroUsize::new(1), + ) + .unwrap(); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + Some(1), + _basic_song_from_database(sqlite_conn, "/random/path").track_number + ); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + None, + _basic_song_from_database(sqlite_conn, "/random/path2").track_number + ); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + None, + _basic_song_from_database(sqlite_conn, "/random/path3").track_number + ); + let sqlite_conn = library.sqlite_conn.lock().unwrap(); + assert_eq!( + None, + _basic_song_from_database(sqlite_conn, "/random/path4").track_number + ); + } + #[test] #[cfg(feature = "ffmpeg")] fn test_library_store_song() {