To see where we left off, see my previous post
Point 2 in my review said that I didn’t like using all of those unwraps – I don’t want my application panicing everywhere.
So, I am going to first focus on this one
api.rs
22. let mut resp = Client::new()
23. .get(&request_url)
24. .basic_auth(&config.api_username, Some(&config.api_password))
25. .header("Accept", "application/json")
26. .send().unwrap();
27. if resp.status().is_success() {
let packet: EnergenieResponse<EnergenieUserProfile> = resp.json().unwrap();
28. Ok(Session {
29. config: config,
30. api_key: packet.data.api_key
31. })
32. } else {
33. Err("Boom")
34. }
The first unwrap is in line 26. So, the first change is to remove it and to get some feedback from the compiler to give us a hint as to what type the .send() returns.
So, this should now read :- .send()
cargo run
Garys-MacBook-Pro:energenie-api garyhome$ cargo run
Compiling energenie-api v0.1.0 (/Users/garyhome/Sync/rust/smarthome/energenie-api)
error[E0599]: no method namedstatus
found for typestd::result::Result<api::reqwest::Response, api::reqwest::Error>
in the current scope
–> energenie-api/src/api.rs:27:17
|
27 | if resp.status().is_success() {
| ^^^^^^
error[E0599]: no method namedjson
found for typestd::result::Result<api::reqwest::Response, api::reqwest::Error>
in the current scope
–> energenie-api/src/api.rs:28:72
|
28 | let packet: EnergenieResponse = resp.json().unwrap();
| ^^^^
error: aborting due to 2 previous errors
For more information about this error, tryrustc --explain E0599
.
error: Could not compileenergenie-api
.
To learn more, run the command again with –verbose.
So, from this feedback, it is suggesting we are now dealing with a std::result::Result<api::reqwest::Response, api::reqwest::Error>
So, a match can be used to branch on the result, so we are pretty much going to take the contents of the if and else statements starting on line 27 – the contents of the if will go in the ‘Ok’ arm of the match and the contents of the else will go in the ‘Err’ arm of the match. We will change the variable name from resp to result and resp will be used in the match. So, the code will look like this :-
17. impl Session {
18. /// Starts a new session
19. pub fn start() -> Result<Session, &'static str> {
20. let config = Config::new();
21. let request_url = format!("{api_base}/users/profile", api_base = &config.api_url);
22. let mut result = Client::new()
23. .get(&request_url)
24. .basic_auth(&config.api_username, Some(&config.api_password))
25. .header("Accept", "application/json")
26 .send();
27. match result {
28. Ok(resp) => {
29. let packet: EnergenieResponse<EnergenieUserProfile> = resp.json().unwrap();
30. Ok(Session {
31. config: config,
32. api_key: packet.data.api_key
33. })
34. },
35. Err(err) => Err("Boom")
36. }
37.
38. }
39. }
Ok, lets get some feedback from the compiler again
cargo run
Garys-MacBook-Pro:energenie-api garyhome$ cargo run
Compiling energenie-api v0.1.0 (/Users/garyhome/Sync/rust/smarthome/energenie-api)
warning: unused variable:err
–> energenie-api/src/api.rs:35:17
|
35 | Err(err) => Err(“Boom”)
| ^^^ help: consider using_err
instead
|
= note: #[warn(unused_variables)] on by default
warning: variable does not need to be mutable
–> energenie-api/src/api.rs:22:13
|
22 | let mut result = Client::new()
| —-^^^^^^
| |
| help: remove thismut
|
= note: #[warn(unused_mut)] on by default
error[E0596]: cannot borrowresp
as mutable, as it is not declared as mutable
–> energenie-api/src/api.rs:29:71
|
28 | Ok(resp) => {
| —- help: consider changing this to be mutable:mut resp
29 | let packet: EnergenieResponse = resp.json().unwrap();
| ^^^^ cannot borrow as mutable
error: aborting due to previous error
For more information about this error, tryrustc --explain E0596
.
error: Could not compileenergenie-api
.
To learn more, run the command again with –verbose.
The compiler is brilliant – it pretty much tells me what errors I have made – so removing a ‘mut’ in one place and adding in another.
You might wonder why a response needs to be mutable when we are only reading it. It is because the response is much like a file – with a pointer that you read from – so the entire response does not have to be represented in one big string which could get hairy on big responses,
So, after removing the mut from the let and adding it to the Ok(resp) – we end up with api.rs looking like this
api.rs
//! The api stuff
extern crate reqwest;
extern crate serde_derive;
extern crate serde_json;
extern crate serde;
use reqwest::Client;
use std::env;
// A Session is where everything begins. My plan (for now) is that we go via the session
// for everything, but we will see how the API develops.
#[derive(Debug)]
pub struct Session {
pub config: Config,
pub api_key: String
}
impl Session {
/// Starts a new session
pub fn start() -> Result<Session, &'static str> {
let config = Config::new();
let request_url = format!("{api_base}/users/profile", api_base = &config.api_url);
let result = Client::new()
.get(&request_url)
.basic_auth(&config.api_username, Some(&config.api_password))
.header("Accept", "application/json")
.send();
match result {
Ok(mut resp) => {
let packet: EnergenieResponse<EnergenieUserProfile> = resp.json().unwrap();
Ok(Session {
config: config,
api_key: packet.data.api_key
})
},
Err(err) => Err("Boom")
}
}
}
// The Config struct is used to gather all of the configuration into one place.
// At the moment, it is a combination of hard coding and environment variables.
// I have not really planned ahead to thing where it is going to get the config from long term
// but, lets learn to walk before we run ?
#[derive(Debug)]
pub struct Config {
api_url: String,
api_password: String,
api_username: String
}
impl Config {
pub fn new() -> Config {
Config {
api_url: String::from("https://mihome4u.co.uk/api/v1"),
api_username: env::var("MIHOME_USERNAME").expect("MIHOME_USERNAME must be set"),
api_password: env::var("MIHOME_PASSWORD").expect("MIHOME_PASSWORD must be set")
}
}
}
// Below are the data structures returned by the API - I am hoping to move these into a different file
#[derive(Deserialize, Debug)]
struct EnergenieUserProfile {
id: i64,
email_address: String,
api_key: String
}
#[derive(Deserialize, Debug)]
struct EnergenieResponse<T> {
status: String,
time: f64,
data: T
}
Ok, lets get some more feedback with cargo run
cargo run
Garys-MacBook-Pro:energenie-api garyhome$ cargo run
Compiling energenie-api v0.1.0 (/Users/garyhome/Sync/rust/smarthome/energenie-api)
warning: unused variable:err
–> energenie-api/src/api.rs:35:17
|
35 | Err(err) => Err(“Boom”)
| ^^^ help: consider using_err
instead
|
= note: #[warn(unused_variables)] on by default
warning: unused variable:err
–> energenie-api/src/main.rs:7:13
|
7 | Err(err) => println!(“It didnt connect”)
| ^^^ help: consider using_err
instead
|
= note: #[warn(unused_variables)] on by default
Finished dev [unoptimized + debuginfo] target(s) in 3.60s Running `/Users/garyhome/Sync/rust/smarthome/target/debug/energenie-api`
It connected, the session looks like this Session { config: Config { api_url: “https://mihome4u.co.uk/api/v1”, api_password: “********”, api_username: “***********” }, api_key: “**********************************” }
Garys-MacBook-Pro:energenie-api garyhome$
Celebration – it works. But, we still have an unwrap in the Ok arm of the match where we call the json function
Ok(mut resp) => {
let packet: EnergenieResponse<EnergenieUserProfile> = resp.json().unwrap();
So, lets take a similar approach. Removing the unwrap and getting feedback from the compiler suggests we are again dealing with a Result type (we could have checked the documentation for reqwest as well of course). So, lets add a match
This is what the outer match looks like now
match result {
Ok(mut resp) => {
match resp.json() {
Ok(json) => {
let packet: EnergenieResponse<EnergenieUserProfile> = json;
Ok(Session {
config: config,
api_key: packet.data.api_key
})
}
Err(err) => Err("The JSON could not be parsed")
}
},
Err(err) => Err("Boom")
}
And this works – excellent. I am struggling with the errors right now, having them returned as strings is a pain as I can’t just pass the original error along if I want to. We can come back to that later though – it’s on our todo list already.
This code still seems a little ‘stepped’ for me and too hard to read. I think it needs refactoring a little – maybe we should have a function that is responsible for parsing a ‘packet’ and using generics to allow for any inner type.
So, lets change the Session implementation as follows.
impl Session {
/// Starts a new session
pub fn start() -> Result<Session, &'static str> {
let config = Config::new();
let request_url = format!("{api_base}/users/profile", api_base = &config.api_url);
let result = Client::new()
.get(&request_url)
.basic_auth(&config.api_username, Some(&config.api_password))
.header("Accept", "application/json")
.send();
match result {
Ok(mut resp) => {
match resp.json() {
Ok(json) => Ok(Session::parse_new_session_response(config, json)),
Err(err) => Err("The JSON could not be parsed")
}
},
Err(err) => Err("Boom")
}
}
fn parse_new_session_response(config: Config, json: EnergenieResponse<EnergenieUserProfile>) -> Session {
Session {
config: config,
api_key: json.data.api_key
}
}
}
Its still a little stepped, but it looks a bit better. I think we could do better by writing a generic function to parse an energenie response with maybe a passed in function or a pointer to a function to decide what to do with the data once parsed.
But, I don’t want to take too big a step at the moment – it works, it’s reasonable – it’s mostly readable – I’m happy with this bit of refactoring. Once we start to implement the remainder of the API, we should notice more patterns – I think the new session is going to be a bit of a special one as we are grabbing a very specific piece of data (the api key) and returning something completely different (a new session).
0 Comments