From b4c7e8e057e4109a758b2e443c98a3221f1b2933 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Fri, 6 Jan 2017 12:50:44 +0100 Subject: [PATCH 1/5] Make main_helper useful with values not from getopts. Applications that gets these values from config file shouldn't have to reinvent the wheel. --- src/main.rs | 6 +++--- src/main_helper.rs | 47 ++++++++++++++++++++++++++++++---------------- 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/main.rs b/src/main.rs index 69fbace4..6eda338c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -32,11 +32,11 @@ fn main() { main_helper::setup_logging(&matches); - let session = main_helper::create_session(&matches); - let credentials = main_helper::get_credentials(&session, &matches); + let session = main_helper::session_from_matches(&matches); + let credentials = main_helper::credentials_from_matches(&session, &matches); session.login(credentials).unwrap(); - let player = main_helper::create_player(&session, &matches); + let player = main_helper::player_from_matches(&session, &matches); let spirc = SpircManager::new(session.clone(), player); let spirc_signal = spirc.clone(); diff --git a/src/main_helper.rs b/src/main_helper.rs index 67e8b2a2..9cdc959e 100644 --- a/src/main_helper.rs +++ b/src/main_helper.rs @@ -55,14 +55,26 @@ pub fn add_player_arguments(opts: &mut getopts::Options) { .optopt("", "device", "Audio device to use. Use '?' to list options", "DEVICE"); } -pub fn create_session(matches: &getopts::Matches) -> Session { +pub fn session_from_matches(matches: &getopts::Matches) -> Session { + create_session(matches.opt_str("n").unwrap(), + matches.opt_str("b").as_ref().map(String::as_ref), + matches.opt_str("c").as_ref().map(String::as_ref), + matches.opt_str("onstart"), + matches.opt_str("onstop")) +} + +pub fn create_session(name: String, + bitrate: Option<&str>, + cache: Option<&str>, + onstart: Option, + onstop: Option) + -> Session { info!("librespot {} ({}). Built on {}.", version::short_sha(), version::commit_date(), version::short_now()); - let name = matches.opt_str("n").unwrap(); - let bitrate = match matches.opt_str("b").as_ref().map(String::as_ref) { + let bitrate = match bitrate { None => Bitrate::Bitrate160, // default value Some("96") => Bitrate::Bitrate96, @@ -74,13 +86,10 @@ pub fn create_session(matches: &getopts::Matches) -> Session { } }; - let cache = matches.opt_str("c").map(|cache_location| { + let cache = cache.map(|cache_location| { Box::new(DefaultCache::new(PathBuf::from(cache_location)).unwrap()) as Box }).unwrap_or_else(|| Box::new(NoCache) as Box); - let onstart = matches.opt_str("onstart"); - let onstop = matches.opt_str("onstop"); - let config = Config { user_agent: version::version_string(), device_name: name, @@ -92,13 +101,16 @@ pub fn create_session(matches: &getopts::Matches) -> Session { Session::new(config, cache) } -pub fn get_credentials(session: &Session, matches: &getopts::Matches) -> Credentials { +pub fn credentials_from_matches(session: &Session, + matches: &getopts::Matches) -> Credentials { + get_credentials(session, matches.opt_str("username"), matches.opt_str("password")) +} + +pub fn get_credentials(session: &Session, username: Option, + password: Option) -> Credentials { let credentials = session.cache().get_credentials(); - match (matches.opt_str("username"), - matches.opt_str("password"), - credentials) { - + match (username, password, credentials) { (Some(username), Some(password), _) => Credentials::with_password(username, password), @@ -122,11 +134,14 @@ pub fn get_credentials(session: &Session, matches: &getopts::Matches) -> Credent } } -pub fn create_player(session: &Session, matches: &getopts::Matches) -> Player { - let backend_name = matches.opt_str("backend"); - let device_name = matches.opt_str("device"); +pub fn player_from_matches(session: &Session, matches: &getopts::Matches) -> Player { + create_player(session, matches.opt_str("backend").as_ref().map(String::as_ref) + , matches.opt_str("device")) +} - let make_backend = find_backend(backend_name.as_ref().map(AsRef::as_ref)); +pub fn create_player(session: &Session, backend_name: Option<&str> + , device_name: Option) -> Player { + let make_backend = find_backend(backend_name); Player::new(session.clone(), move || { make_backend(device_name.as_ref().map(AsRef::as_ref)) From 28aed0d18b0d165cd3a99f10b42c313834a59a72 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Fri, 6 Jan 2017 12:56:34 +0100 Subject: [PATCH 2/5] Give Bitrate copy semantics. For such a simple type, which will likely never change to be un-copyable, it's nicer to have copy semantics. --- src/session.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/session.rs b/src/session.rs index 74a50b10..b8054dd2 100644 --- a/src/session.rs +++ b/src/session.rs @@ -28,6 +28,7 @@ use version; use stream; +#[derive(Clone, Copy)] pub enum Bitrate { Bitrate96, Bitrate160, From 1f32efce836b032a3b9f334c74fa6604fb90cfb9 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Fri, 6 Jan 2017 17:06:14 +0100 Subject: [PATCH 3/5] Revert "Make main_helper useful with values not from getopts." This reverts commit b4c7e8e057e4109a758b2e443c98a3221f1b2933. --- src/main.rs | 6 +++--- src/main_helper.rs | 47 ++++++++++++++++------------------------------ 2 files changed, 19 insertions(+), 34 deletions(-) diff --git a/src/main.rs b/src/main.rs index 6eda338c..69fbace4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -32,11 +32,11 @@ fn main() { main_helper::setup_logging(&matches); - let session = main_helper::session_from_matches(&matches); - let credentials = main_helper::credentials_from_matches(&session, &matches); + let session = main_helper::create_session(&matches); + let credentials = main_helper::get_credentials(&session, &matches); session.login(credentials).unwrap(); - let player = main_helper::player_from_matches(&session, &matches); + let player = main_helper::create_player(&session, &matches); let spirc = SpircManager::new(session.clone(), player); let spirc_signal = spirc.clone(); diff --git a/src/main_helper.rs b/src/main_helper.rs index 9cdc959e..67e8b2a2 100644 --- a/src/main_helper.rs +++ b/src/main_helper.rs @@ -55,26 +55,14 @@ pub fn add_player_arguments(opts: &mut getopts::Options) { .optopt("", "device", "Audio device to use. Use '?' to list options", "DEVICE"); } -pub fn session_from_matches(matches: &getopts::Matches) -> Session { - create_session(matches.opt_str("n").unwrap(), - matches.opt_str("b").as_ref().map(String::as_ref), - matches.opt_str("c").as_ref().map(String::as_ref), - matches.opt_str("onstart"), - matches.opt_str("onstop")) -} - -pub fn create_session(name: String, - bitrate: Option<&str>, - cache: Option<&str>, - onstart: Option, - onstop: Option) - -> Session { +pub fn create_session(matches: &getopts::Matches) -> Session { info!("librespot {} ({}). Built on {}.", version::short_sha(), version::commit_date(), version::short_now()); - let bitrate = match bitrate { + let name = matches.opt_str("n").unwrap(); + let bitrate = match matches.opt_str("b").as_ref().map(String::as_ref) { None => Bitrate::Bitrate160, // default value Some("96") => Bitrate::Bitrate96, @@ -86,10 +74,13 @@ pub fn create_session(name: String, } }; - let cache = cache.map(|cache_location| { + let cache = matches.opt_str("c").map(|cache_location| { Box::new(DefaultCache::new(PathBuf::from(cache_location)).unwrap()) as Box }).unwrap_or_else(|| Box::new(NoCache) as Box); + let onstart = matches.opt_str("onstart"); + let onstop = matches.opt_str("onstop"); + let config = Config { user_agent: version::version_string(), device_name: name, @@ -101,16 +92,13 @@ pub fn create_session(name: String, Session::new(config, cache) } -pub fn credentials_from_matches(session: &Session, - matches: &getopts::Matches) -> Credentials { - get_credentials(session, matches.opt_str("username"), matches.opt_str("password")) -} - -pub fn get_credentials(session: &Session, username: Option, - password: Option) -> Credentials { +pub fn get_credentials(session: &Session, matches: &getopts::Matches) -> Credentials { let credentials = session.cache().get_credentials(); - match (username, password, credentials) { + match (matches.opt_str("username"), + matches.opt_str("password"), + credentials) { + (Some(username), Some(password), _) => Credentials::with_password(username, password), @@ -134,14 +122,11 @@ pub fn get_credentials(session: &Session, username: Option, } } -pub fn player_from_matches(session: &Session, matches: &getopts::Matches) -> Player { - create_player(session, matches.opt_str("backend").as_ref().map(String::as_ref) - , matches.opt_str("device")) -} +pub fn create_player(session: &Session, matches: &getopts::Matches) -> Player { + let backend_name = matches.opt_str("backend"); + let device_name = matches.opt_str("device"); -pub fn create_player(session: &Session, backend_name: Option<&str> - , device_name: Option) -> Player { - let make_backend = find_backend(backend_name); + let make_backend = find_backend(backend_name.as_ref().map(AsRef::as_ref)); Player::new(session.clone(), move || { make_backend(device_name.as_ref().map(AsRef::as_ref)) From f11310581c6a98882b1be413ed3679b9d7296d55 Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Fri, 6 Jan 2017 17:09:57 +0100 Subject: [PATCH 4/5] Add standard traits to Bitrate. --- src/session.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/session.rs b/src/session.rs index b8054dd2..1965ec7f 100644 --- a/src/session.rs +++ b/src/session.rs @@ -28,7 +28,7 @@ use version; use stream; -#[derive(Clone, Copy)] +#[derive(Clone, Copy, Debug, PartialOrd, Ord, PartialEq, Eq)] pub enum Bitrate { Bitrate96, Bitrate160, From 560d4e1c4ce52cda41286a5dc8f28790dc6bbd2c Mon Sep 17 00:00:00 2001 From: Simon Persson Date: Fri, 6 Jan 2017 17:18:41 +0100 Subject: [PATCH 5/5] Move get_credentials to authentication module. --- src/authentication/mod.rs | 33 ++++++++++++++++++++++++++++++++- src/main.rs | 4 +++- src/main_helper.rs | 33 --------------------------------- 3 files changed, 35 insertions(+), 35 deletions(-) diff --git a/src/authentication/mod.rs b/src/authentication/mod.rs index fbb47a00..7fec380f 100644 --- a/src/authentication/mod.rs +++ b/src/authentication/mod.rs @@ -6,13 +6,16 @@ use crypto::hmac::Hmac; use crypto::pbkdf2::pbkdf2; use crypto::sha1::Sha1; use protobuf::ProtobufEnum; +use rpassword; use serde; use serde_json; -use std::io::{self, Read, Write}; +use std::io::{self, stderr, Read, Write}; use std::fs::File; use std::path::Path; use rustc_serialize::base64::{self, FromBase64, ToBase64}; +use session::Session; + use protocol::authentication::AuthenticationType; #[derive(Debug, Clone)] @@ -170,3 +173,31 @@ fn deserialize_base64(de: &mut D) -> Result, D::Error> mod discovery; pub use self::discovery::discovery_login; + +pub fn get_credentials(session: &Session, username: Option, password: Option) -> Credentials { + let credentials = session.cache().get_credentials(); + + match (username, password, credentials) { + + (Some(username), Some(password), _) + => Credentials::with_password(username, password), + + (Some(ref username), _, Some(ref credentials)) if *username == credentials.username + => credentials.clone(), + + (Some(username), None, _) => { + write!(stderr(), "Password for {}: ", username).unwrap(); + stderr().flush().unwrap(); + let password = rpassword::read_password().unwrap(); + Credentials::with_password(username.clone(), password) + } + + (None, _, Some(credentials)) + => credentials, + + (None, _, None) => { + info!("No username provided and no stored credentials, starting discovery ..."); + discovery_login(&session.config().device_name, session.device_id()).unwrap() + } + } +} diff --git a/src/main.rs b/src/main.rs index 69fbace4..b9c41654 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,6 +8,7 @@ use std::thread; use librespot::spirc::SpircManager; use librespot::main_helper; +use librespot::authentication::get_credentials; fn usage(program: &str, opts: &getopts::Options) -> String { let brief = format!("Usage: {} [options]", program); @@ -33,7 +34,8 @@ fn main() { main_helper::setup_logging(&matches); let session = main_helper::create_session(&matches); - let credentials = main_helper::get_credentials(&session, &matches); + let credentials = get_credentials(&session, matches.opt_str("username"), + matches.opt_str("password")); session.login(credentials).unwrap(); let player = main_helper::create_player(&session, &matches); diff --git a/src/main_helper.rs b/src/main_helper.rs index 67e8b2a2..e42660b7 100644 --- a/src/main_helper.rs +++ b/src/main_helper.rs @@ -1,13 +1,10 @@ use env_logger::LogBuilder; use getopts; -use rpassword; use std::env; -use std::io::{stderr, Write}; use std::path::PathBuf; use std::process::exit; use audio_backend::{BACKENDS, Sink}; -use authentication::{Credentials, discovery_login}; use cache::{Cache, DefaultCache, NoCache}; use player::Player; use session::{Bitrate, Config, Session}; @@ -92,36 +89,6 @@ pub fn create_session(matches: &getopts::Matches) -> Session { Session::new(config, cache) } -pub fn get_credentials(session: &Session, matches: &getopts::Matches) -> Credentials { - let credentials = session.cache().get_credentials(); - - match (matches.opt_str("username"), - matches.opt_str("password"), - credentials) { - - (Some(username), Some(password), _) - => Credentials::with_password(username, password), - - (Some(ref username), _, Some(ref credentials)) if *username == credentials.username - => credentials.clone(), - - (Some(username), None, _) => { - write!(stderr(), "Password for {}: ", username).unwrap(); - stderr().flush().unwrap(); - let password = rpassword::read_password().unwrap(); - Credentials::with_password(username.clone(), password) - } - - (None, _, Some(credentials)) - => credentials, - - (None, _, None) => { - info!("No username provided and no stored credentials, starting discovery ..."); - discovery_login(&session.config().device_name, session.device_id()).unwrap() - } - } -} - pub fn create_player(session: &Session, matches: &getopts::Matches) -> Player { let backend_name = matches.opt_str("backend"); let device_name = matches.opt_str("device");