Merge pull request #166 from librespot-org/apireview

API review

connect/src/spirc.rs

@@ -680,7 +680,7 @@ impl SpircTask {
         let index = self.state.get_playing_track_index();
         let track = {
             let gid = self.state.get_track()[index as usize].get_gid();
-            SpotifyId::from_raw(gid)
+            SpotifyId::from_raw(gid).unwrap()
         };
         let position = self.state.get_position_ms();
 

core/src/authentication.rs

@@ -7,11 +7,11 @@ use crypto::hmac::Hmac;
 use crypto::pbkdf2::pbkdf2;
 use crypto::sha1::Sha1;
 use protobuf::ProtobufEnum;
-use rpassword;
 use serde;
 use serde_json;
 use std::fs::File;
-use std::io::{self, stderr, Read, Write};
+use std::io::{self, Read, Write};
+use std::ops::FnOnce;
 use std::path::Path;
 
 use protocol::authentication::AuthenticationType;
@@ -180,10 +180,11 @@ where
     base64::decode(&v).map_err(|e| serde::de::Error::custom(e.to_string()))
 }
 
-pub fn get_credentials(
+pub fn get_credentials<F: FnOnce(&String) -> String>(
     username: Option<String>,
     password: Option<String>,
     cached_credentials: Option<Credentials>,
+    prompt: F,
 ) -> Option<Credentials> {
     match (username, password, cached_credentials) {
         (Some(username), Some(password), _) => Some(Credentials::with_password(username, password)),
@@ -192,12 +193,10 @@ pub fn get_credentials(
             Some(credentials.clone())
         }
 
-        (Some(username), None, _) => {
+        (Some(username), None, _) => Some(Credentials::with_password(
-            write!(stderr(), "Password for {}: ", username).unwrap();
+            username.clone(),
-            stderr().flush().unwrap();
+            prompt(&username),
-            let password = rpassword::read_password().unwrap();
+        )),
-            Some(Credentials::with_password(username.clone(), password))
-        }
 
         (None, _, Some(credentials)) => Some(credentials),
 

core/src/spotify_id.rs

@@ -9,45 +9,54 @@ use std::ascii::AsciiExt;
 #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct SpotifyId(u128);
 
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+pub struct SpotifyIdError;
+
 const BASE62_DIGITS: &'static [u8] = b"0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ";
 const BASE16_DIGITS: &'static [u8] = b"0123456789abcdef";
 
 impl SpotifyId {
-    pub fn from_base16(id: &str) -> SpotifyId {
+    pub fn from_base16(id: &str) -> Result<SpotifyId, SpotifyIdError> {
-        assert!(id.is_ascii());
         let data = id.as_bytes();
 
         let mut n: u128 = u128::zero();
         for c in data {
-            let d = BASE16_DIGITS.iter().position(|e| e == c).unwrap() as u8;
+            let d = match BASE16_DIGITS.iter().position(|e| e == c) {
+                None => return Err(SpotifyIdError),
+                Some(x) => x as u8,
+            };
             n = n * u128::from(16);
             n = n + u128::from(d);
         }
 
-        SpotifyId(n)
+        Ok(SpotifyId(n))
     }
 
-    pub fn from_base62(id: &str) -> SpotifyId {
+    pub fn from_base62(id: &str) -> Result<SpotifyId, SpotifyIdError> {
-        assert!(id.is_ascii());
         let data = id.as_bytes();
 
         let mut n: u128 = u128::zero();
         for c in data {
-            let d = BASE62_DIGITS.iter().position(|e| e == c).unwrap() as u8;
+            let d = match BASE62_DIGITS.iter().position(|e| e == c) {
+                None => return Err(SpotifyIdError),
+                Some(x) => x as u8,
+            };
             n = n * u128::from(62);
             n = n + u128::from(d);
         }
 
-        SpotifyId(n)
+        Ok(SpotifyId(n))
     }
 
-    pub fn from_raw(data: &[u8]) -> SpotifyId {
+    pub fn from_raw(data: &[u8]) -> Result<SpotifyId, SpotifyIdError> {
-        assert_eq!(data.len(), 16);
+        if data.len() != 16 {
+            return Err(SpotifyIdError);
+        };
 
         let high = BigEndian::read_u64(&data[0..8]);
         let low = BigEndian::read_u64(&data[8..16]);
 
-        SpotifyId(u128::from_parts(high, low))
+        Ok(SpotifyId(u128::from_parts(high, low)))
     }
 
     pub fn to_base16(&self) -> String {

examples/play.rs

@@ -28,7 +28,7 @@ fn main() {
     let password = args[2].to_owned();
     let credentials = Credentials::with_password(username, password);
 
-    let track = SpotifyId::from_base62(&args[3]);
+    let track = SpotifyId::from_base62(&args[3]).unwrap();
 
     let backend = audio_backend::find(None).unwrap();
 

metadata/src/lib.rs

@@ -113,7 +113,7 @@ impl Metadata for Track {
         let artists = msg.get_artist()
             .iter()
             .filter(|artist| artist.has_gid())
-            .map(|artist| SpotifyId::from_raw(artist.get_gid()))
+            .map(|artist| SpotifyId::from_raw(artist.get_gid()).unwrap())
             .collect::<Vec<_>>();
 
         let files = msg.get_file()
@@ -127,15 +127,15 @@ impl Metadata for Track {
             .collect();
 
         Track {
-            id: SpotifyId::from_raw(msg.get_gid()),
+            id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
             name: msg.get_name().to_owned(),
             duration: msg.get_duration(),
-            album: SpotifyId::from_raw(msg.get_album().get_gid()),
+            album: SpotifyId::from_raw(msg.get_album().get_gid()).unwrap(),
             artists: artists,
             files: files,
             alternatives: msg.get_alternative()
                 .iter()
-                .map(|alt| SpotifyId::from_raw(alt.get_gid()))
+                .map(|alt| SpotifyId::from_raw(alt.get_gid()).unwrap())
                 .collect(),
             available: parse_restrictions(msg.get_restriction(), &country, "premium"),
         }
@@ -153,14 +153,14 @@ impl Metadata for Album {
         let artists = msg.get_artist()
             .iter()
             .filter(|artist| artist.has_gid())
-            .map(|artist| SpotifyId::from_raw(artist.get_gid()))
+            .map(|artist| SpotifyId::from_raw(artist.get_gid()).unwrap())
             .collect::<Vec<_>>();
 
         let tracks = msg.get_disc()
             .iter()
             .flat_map(|disc| disc.get_track())
             .filter(|track| track.has_gid())
-            .map(|track| SpotifyId::from_raw(track.get_gid()))
+            .map(|track| SpotifyId::from_raw(track.get_gid()).unwrap())
             .collect::<Vec<_>>();
 
         let covers = msg.get_cover_group()
@@ -175,7 +175,7 @@ impl Metadata for Album {
             .collect::<Vec<_>>();
 
         Album {
-            id: SpotifyId::from_raw(msg.get_gid()),
+            id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
             name: msg.get_name().to_owned(),
             artists: artists,
             tracks: tracks,
@@ -202,13 +202,13 @@ impl Metadata for Artist {
                 .get_track()
                 .iter()
                 .filter(|track| track.has_gid())
-                .map(|track| SpotifyId::from_raw(track.get_gid()))
+                .map(|track| SpotifyId::from_raw(track.get_gid()).unwrap())
                 .collect::<Vec<_>>(),
             None => Vec::new(),
         };
 
         Artist {
-            id: SpotifyId::from_raw(msg.get_gid()),
+            id: SpotifyId::from_raw(msg.get_gid()).unwrap(),
             name: msg.get_name().to_owned(),
             top_tracks: top_tracks,
         }

src/main.rs

@@ -3,6 +3,7 @@ extern crate env_logger;
 extern crate futures;
 extern crate getopts;
 extern crate librespot;
+extern crate rpassword;
 extern crate tokio_core;
 extern crate tokio_io;
 extern crate tokio_signal;
@@ -177,10 +178,17 @@ fn setup(args: &[String]) -> Setup {
     let credentials = {
         let cached_credentials = cache.as_ref().and_then(Cache::credentials);
 
+        let password = |username: &String| -> String {
+            write!(stderr(), "Password for {}: ", username).unwrap();
+            stderr().flush().unwrap();
+            rpassword::read_password().unwrap()
+        };
+
         get_credentials(
             matches.opt_str("username"),
             matches.opt_str("password"),
-            cached_credentials
+            cached_credentials,
+            password
         )
     };