From bd537b8134ff566a6b7cdc551534a982335c57df Mon Sep 17 00:00:00 2001 From: Antoine Gersant Date: Sun, 9 Jul 2017 23:32:02 -0700 Subject: [PATCH] Simplified connection API --- src/api.rs | 4 +--- src/config.rs | 62 ++++++++++++++++++++++-------------------------- src/db/mod.rs | 11 ++++++--- src/ddns.rs | 4 +--- src/index.rs | 63 +++++++++++++++++-------------------------------- src/playlist.rs | 37 ++++++++++------------------- src/user.rs | 14 ++++------- src/vfs.rs | 4 +--- 8 files changed, 79 insertions(+), 120 deletions(-) diff --git a/src/api.rs b/src/api.rs index f88cca4..1488129 100644 --- a/src/api.rs +++ b/src/api.rs @@ -48,9 +48,7 @@ fn get_auth_secret(db: &T) -> Result { use self::misc_settings::dsl::*; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - let misc: MiscSettings = misc_settings.get_result(connection)?; + let misc: MiscSettings = misc_settings.get_result(connection.deref())?; Ok(misc.auth_secret.to_owned()) } diff --git a/src/config.rs b/src/config.rs index 6dede54..8d3f815 100644 --- a/src/config.rs +++ b/src/config.rs @@ -78,8 +78,6 @@ pub fn read(db: &T) -> Result use self::ddns_config::dsl::*; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let mut config = Config { album_art_pattern: None, @@ -91,18 +89,18 @@ pub fn read(db: &T) -> Result let (art_pattern, sleep_duration) = misc_settings .select((index_album_art_pattern, index_sleep_duration_seconds)) - .get_result(connection)?; + .get_result(connection.deref())?; config.album_art_pattern = Some(art_pattern); config.reindex_every_n_seconds = Some(sleep_duration); let mount_dirs = mount_points .select((source, name)) - .get_results(connection)?; + .get_results(connection.deref())?; config.mount_dirs = Some(mount_dirs); let found_users: Vec<(String, i32)> = users::table .select((users::columns::name, users::columns::admin)) - .get_results(connection)?; + .get_results(connection.deref())?; config.users = Some(found_users .into_iter() .map(|(n, a)| { @@ -116,7 +114,7 @@ pub fn read(db: &T) -> Result let ydns = ddns_config .select((host, username, password)) - .get_result(connection)?; + .get_result(connection.deref())?; config.ydns = Some(ydns); Ok(config) @@ -127,14 +125,13 @@ fn reset(db: &T) -> Result<()> { use self::ddns_config::dsl::*; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - diesel::delete(mount_points::table).execute(connection)?; - diesel::delete(users::table).execute(connection)?; + diesel::delete(mount_points::table) + .execute(connection.deref())?; + diesel::delete(users::table).execute(connection.deref())?; diesel::update(ddns_config) .set((host.eq(""), username.eq(""), password.eq(""))) - .execute(connection)?; + .execute(connection.deref())?; Ok(()) } @@ -150,20 +147,19 @@ pub fn amend(db: &T, new_config: &Config) -> Result<()> where T: ConnectionSource { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); if let Some(ref mount_dirs) = new_config.mount_dirs { - diesel::delete(mount_points::table).execute(connection)?; + diesel::delete(mount_points::table) + .execute(connection.deref())?; diesel::insert(mount_dirs) .into(mount_points::table) - .execute(connection)?; + .execute(connection.deref())?; } if let Some(ref config_users) = new_config.users { let old_usernames: Vec = users::table .select(users::name) - .get_results(connection)?; + .get_results(connection.deref())?; // Delete users that are not in new list // Delete users that have a new password @@ -175,7 +171,7 @@ pub fn amend(db: &T, new_config: &Config) -> Result<()> }) .collect::<_>(); diesel::delete(users::table.filter(users::name.eq_any(&delete_usernames))) - .execute(connection)?; + .execute(connection.deref())?; // Insert users that have a new password let insert_users: Vec<&ConfigUser> = config_users @@ -186,27 +182,27 @@ pub fn amend(db: &T, new_config: &Config) -> Result<()> let new_user = User::new(&config_user.name, &config_user.password, config_user.admin); diesel::insert(&new_user) .into(users::table) - .execute(connection)?; + .execute(connection.deref())?; } // Grant admin rights for ref user in config_users { diesel::update(users::table.filter(users::name.eq(&user.name))) .set(users::admin.eq(user.admin as i32)) - .execute(connection)?; + .execute(connection.deref())?; } } if let Some(sleep_duration) = new_config.reindex_every_n_seconds { diesel::update(misc_settings::table) .set(misc_settings::index_sleep_duration_seconds.eq(sleep_duration as i32)) - .execute(connection)?; + .execute(connection.deref())?; } if let Some(ref album_art_pattern) = new_config.album_art_pattern { diesel::update(misc_settings::table) .set(misc_settings::index_album_art_pattern.eq(album_art_pattern)) - .execute(connection)?; + .execute(connection.deref())?; } if let Some(ref ydns) = new_config.ydns { @@ -215,7 +211,7 @@ pub fn amend(db: &T, new_config: &Config) -> Result<()> .set((host.eq(ydns.host.clone()), username.eq(ydns.username.clone()), password.eq(ydns.password.clone()))) - .execute(connection)?; + .execute(connection.deref())?; } Ok(()) @@ -314,12 +310,10 @@ fn test_amend_preserve_password_hashes() { { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); initial_hash = users .select(password_hash) .filter(name.eq("Teddy🐻")) - .get_result(connection) + .get_result(connection.deref()) .unwrap(); } @@ -343,12 +337,10 @@ fn test_amend_preserve_password_hashes() { { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); new_hash = users .select(password_hash) .filter(name.eq("Teddy🐻")) - .get_result(connection) + .get_result(connection.deref()) .unwrap(); } @@ -377,9 +369,10 @@ fn test_toggle_admin() { { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - let is_admin: i32 = users.select(admin).get_result(connection).unwrap(); + let is_admin: i32 = users + .select(admin) + .get_result(connection.deref()) + .unwrap(); assert_eq!(is_admin, 1); } @@ -398,9 +391,10 @@ fn test_toggle_admin() { { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - let is_admin: i32 = users.select(admin).get_result(connection).unwrap(); + let is_admin: i32 = users + .select(admin) + .get_result(connection.deref()) + .unwrap(); assert_eq!(is_admin, 0); } } diff --git a/src/db/mod.rs b/src/db/mod.rs index ba94673..adf66c3 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -4,7 +4,7 @@ use diesel::prelude::*; use diesel::sqlite::SqliteConnection; use std::fs; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, MutexGuard}; use errors::*; @@ -17,7 +17,8 @@ const DB_MIGRATIONS_PATH: &'static str = "src/db/migrations"; embed_migrations!("src/db/migrations"); pub trait ConnectionSource { - fn get_connection(&self) -> Arc>; + fn get_connection(&self) -> MutexGuard; + fn get_connection_mutex(&self) -> Arc>; } pub struct DB { @@ -66,7 +67,11 @@ impl DB { } impl ConnectionSource for DB { - fn get_connection(&self) -> Arc> { + fn get_connection(&self) -> MutexGuard { + self.connection.lock().unwrap() + } + + fn get_connection_mutex(&self) -> Arc> { self.connection.clone() } } diff --git a/src/ddns.rs b/src/ddns.rs index 7a9c5f1..85c241c 100644 --- a/src/ddns.rs +++ b/src/ddns.rs @@ -26,11 +26,9 @@ impl DDNSConfigSource for DB { fn get_ddns_config(&self) -> errors::Result { use self::ddns_config::dsl::*; let connection = self.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); Ok(ddns_config .select((host, username, password)) - .get_result(connection)?) + .get_result(connection.deref())?) } } diff --git a/src/index.rs b/src/index.rs index 8ee969c..2c4960c 100644 --- a/src/index.rs +++ b/src/index.rs @@ -295,9 +295,9 @@ fn clean(db: &T) -> Result<()> let all_songs: Vec; { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - all_songs = songs::table.select(songs::path).load(connection)?; + all_songs = songs::table + .select(songs::path) + .load(connection.deref())?; } let missing_songs = all_songs @@ -310,11 +310,9 @@ fn clean(db: &T) -> Result<()> { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); for chunk in missing_songs[..].chunks(INDEX_BUILDING_CLEAN_BUFFER_SIZE) { diesel::delete(songs::table.filter(songs::path.eq_any(chunk))) - .execute(connection)?; + .execute(connection.deref())?; } } @@ -324,11 +322,9 @@ fn clean(db: &T) -> Result<()> let all_directories: Vec; { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); all_directories = directories::table .select(directories::path) - .load(connection)?; + .load(connection.deref())?; } let missing_directories = all_directories @@ -341,11 +337,9 @@ fn clean(db: &T) -> Result<()> { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); for chunk in missing_directories[..].chunks(INDEX_BUILDING_CLEAN_BUFFER_SIZE) { diesel::delete(directories::table.filter(directories::path.eq_any(chunk))) - .execute(connection)?; + .execute(connection.deref())?; } } } @@ -358,17 +352,16 @@ fn populate(db: &T) -> Result<()> { let vfs = db.get_vfs()?; let mount_points = vfs.get_mount_points(); - let connection = db.get_connection(); let album_art_pattern; { - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - let settings: MiscSettings = misc_settings::table.get_result(connection)?; + let connection = db.get_connection(); + let settings: MiscSettings = misc_settings::table.get_result(connection.deref())?; album_art_pattern = Regex::new(&settings.index_album_art_pattern)?; } - let mut builder = IndexBuilder::new(&connection, album_art_pattern)?; + let connection_mutex = db.get_connection_mutex(); + let mut builder = IndexBuilder::new(connection_mutex.deref(), album_art_pattern)?; for (_, target) in mount_points { builder.populate_directory(None, target.as_path())?; } @@ -433,10 +426,8 @@ pub fn self_trigger(db: &T, command_buffer: Arc>>) let sleep_duration; { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let settings: Result = misc_settings::table - .get_result(connection) + .get_result(connection.deref()) .map_err(|e| e.into()); if let Err(ref e) = settings { println!("Could not retrieve index sleep duration: {}", e); @@ -483,14 +474,12 @@ pub fn browse(db: &T, virtual_path: &Path) -> Result> let mut output = Vec::new(); let vfs = db.get_vfs()?; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); if virtual_path.components().count() == 0 { // Browse top-level let real_directories: Vec = directories::table .filter(directories::parent.is_null()) - .load(connection)?; + .load(connection.deref())?; let virtual_directories = real_directories .into_iter() .filter_map(|s| virtualize_directory(&vfs, s)); @@ -506,7 +495,7 @@ pub fn browse(db: &T, virtual_path: &Path) -> Result> let real_directories: Vec = directories::table .filter(directories::parent.eq(&real_path_string)) .order(sql::("path COLLATE NOCASE ASC")) - .load(connection)?; + .load(connection.deref())?; let virtual_directories = real_directories .into_iter() .filter_map(|s| virtualize_directory(&vfs, s)); @@ -515,7 +504,7 @@ pub fn browse(db: &T, virtual_path: &Path) -> Result> let real_songs: Vec = songs::table .filter(songs::parent.eq(&real_path_string)) .order(sql::("path COLLATE NOCASE ASC")) - .load(connection)?; + .load(connection.deref())?; let virtual_songs = real_songs .into_iter() .filter_map(|s| virtualize_song(&vfs, s)); @@ -531,11 +520,11 @@ pub fn flatten(db: &T, virtual_path: &Path) -> Result> use self::songs::dsl::*; let vfs = db.get_vfs()?; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let real_path = vfs.virtual_to_real(virtual_path)?; let like_path = real_path.as_path().to_string_lossy().into_owned() + "%"; - let real_songs: Vec = songs.filter(path.like(&like_path)).load(connection)?; + let real_songs: Vec = songs + .filter(path.like(&like_path)) + .load(connection.deref())?; let virtual_songs = real_songs .into_iter() .filter_map(|s| virtualize_song(&vfs, s)); @@ -548,13 +537,11 @@ pub fn get_random_albums(db: &T, count: i64) -> Result> use self::directories::dsl::*; let vfs = db.get_vfs()?; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let real_directories = directories .filter(album.is_not_null()) .limit(count) .order(random) - .load(connection)?; + .load(connection.deref())?; let virtual_directories = real_directories .into_iter() .filter_map(|s| virtualize_directory(&vfs, s)); @@ -567,13 +554,11 @@ pub fn get_recent_albums(db: &T, count: i64) -> Result> use self::directories::dsl::*; let vfs = db.get_vfs()?; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let real_directories: Vec = directories .filter(album.is_not_null()) .order(date_added.desc()) .limit(count) - .load(connection)?; + .load(connection.deref())?; let virtual_directories = real_directories .into_iter() .filter_map(|s| virtualize_directory(&vfs, s)); @@ -587,10 +572,8 @@ fn test_populate() { update(&db).unwrap(); // Check that subsequent updates don't run into conflicts let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - let all_directories: Vec = directories::table.load(connection).unwrap(); - let all_songs: Vec = songs::table.load(connection).unwrap(); + let all_directories: Vec = directories::table.load(connection.deref()).unwrap(); + let all_songs: Vec = songs::table.load(connection.deref()).unwrap(); assert_eq!(all_directories.len(), 5); assert_eq!(all_songs.len(), 12); } @@ -613,11 +596,9 @@ fn test_metadata() { update(&db).unwrap(); let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let songs: Vec = songs::table .filter(songs::title.eq("シャーベット (Sherbet)")) - .load(connection) + .load(connection.deref()) .unwrap(); assert_eq!(songs.len(), 1); diff --git a/src/playlist.rs b/src/playlist.rs index 74740f0..9bd7d25 100644 --- a/src/playlist.rs +++ b/src/playlist.rs @@ -52,8 +52,6 @@ fn list_playlists(owner: &str, db: &T) -> Result> where T: ConnectionSource + VFSSource { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let user: User; { @@ -61,14 +59,14 @@ fn list_playlists(owner: &str, db: &T) -> Result> user = users .filter(name.eq(owner)) .select((id, name)) - .first(connection)?; + .first(connection.deref())?; } { use self::playlists::dsl::*; let found_playlists: Vec = Playlist::belonging_to(&user) .select(name) - .load(connection)?; + .load(connection.deref())?; Ok(found_playlists) } } @@ -83,8 +81,6 @@ fn save_playlist(name: &str, owner: &str, content: &Vec, db: &T) -> R { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); // Find owner { @@ -92,7 +88,7 @@ fn save_playlist(name: &str, owner: &str, content: &Vec, db: &T) -> R user = users .filter(name.eq(owner)) .select((id, name)) - .get_result(connection)?; + .get_result(connection.deref())?; } // Create playlist @@ -102,19 +98,19 @@ fn save_playlist(name: &str, owner: &str, content: &Vec, db: &T) -> R }; diesel::insert(&new_playlist) - .into(playlists::table) - .execute(connection)?; + .into(playlists::table) + .execute(connection.deref())?; { use self::playlists::dsl::*; playlist = playlists .filter(name.eq(name).and(owner.eq(user.id))) - .get_result(connection)?; + .get_result(connection.deref())?; } // Delete old content (if any) let old_songs = PlaylistSong::belonging_to(&playlist); - diesel::delete(old_songs).execute(connection)?; + diesel::delete(old_songs).execute(connection.deref())?; } // Insert content @@ -136,12 +132,9 @@ fn save_playlist(name: &str, owner: &str, content: &Vec, db: &T) -> R { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - diesel::insert(&new_songs) .into(playlist_songs::table) - .execute(connection)?; + .execute(connection.deref())?; } Ok(()) @@ -157,8 +150,6 @@ fn read_playlist(playlist_name: &str, owner: &str, db: &T) -> Result(playlist_name: &str, owner: &str, db: &T) -> Result(playlist_name: &str, owner: &str, db: &T) -> Result(playlist_name: &str, owner: &str, db: &T) -> Result<()> where T: ConnectionSource + VFSSource { let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let user: User; { @@ -200,13 +189,13 @@ fn delete_playlist(playlist_name: &str, owner: &str, db: &T) -> Result<()> user = users .filter(name.eq(owner)) .select((id, name)) - .first(connection)?; + .first(connection.deref())?; } { use self::playlists::dsl::*; let q = Playlist::belonging_to(&user).filter(name.eq(playlist_name)); - diesel::delete(q).execute(connection)?; + diesel::delete(q).execute(connection.deref())?; } Ok(()) diff --git a/src/user.rs b/src/user.rs index 0f02861..8f6e983 100644 --- a/src/user.rs +++ b/src/user.rs @@ -60,12 +60,10 @@ pub fn auth(db: &T, username: &str, password: &str) -> Result { use db::users::dsl::*; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let user: QueryResult = users .select((name, password_salt, password_hash, admin)) .filter(name.eq(username)) - .get_result(connection); + .get_result(connection.deref()); match user { Err(diesel::result::Error::NotFound) => Ok(false), Ok(u) => Ok(u.verify_password(password)), @@ -78,9 +76,9 @@ pub fn count(db: &T) -> Result { use db::users::dsl::*; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); - Ok(users.select(expression::count(name)).first(connection)?) + Ok(users + .select(expression::count(name)) + .first(connection.deref())?) } pub fn is_admin(db: &T, username: &str) -> Result @@ -88,11 +86,9 @@ pub fn is_admin(db: &T, username: &str) -> Result { use db::users::dsl::*; let connection = db.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let is_admin: i32 = users .filter(name.eq(username)) .select(admin) - .get_result(connection)?; + .get_result(connection.deref())?; Ok(is_admin != 0) } diff --git a/src/vfs.rs b/src/vfs.rs index 3dce6cf..e537202 100644 --- a/src/vfs.rs +++ b/src/vfs.rs @@ -17,11 +17,9 @@ impl VFSSource for DB { use self::mount_points::dsl::*; let mut vfs = VFS::new(); let connection = self.get_connection(); - let connection = connection.lock().unwrap(); - let connection = connection.deref(); let points: Vec = mount_points .select((source, name)) - .get_results(connection)?; + .get_results(connection.deref())?; for point in points { vfs.mount(&Path::new(&point.source), &point.name)?; }