diff mbox

jenkins: refactor API HTTP GET boilerplate into new method

Message ID 1466038776-7096-1-git-send-email-andrew.donnellan@au1.ibm.com
State Superseded
Delegated to: Russell Currey
Headers show

Commit Message

Andrew Donnellan June 16, 2016, 12:59 a.m. UTC 
All our side-effect-free Jenkins API calls follow the same pattern of
creating a hyper Client, making a GET request to the relevant JSON API
endpoint, and decoding the returned JSON into a JSON Object (right now the
top-level data types are all Objects).

Refactor this boilerplate into a new method, get_api_json_object().

Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

---

Build tested only.
---
 src/jenkins.rs | 31 ++++++++++++-------------------
 1 file changed, 12 insertions(+), 19 deletions(-)

Comments

Russell Currey June 16, 2016, 1:35 a.m. UTC | #1 
On Thu, 2016-06-16 at 10:59 +1000, Andrew Donnellan wrote:
> All our side-effect-free Jenkins API calls follow the same pattern of
> creating a hyper Client, making a GET request to the relevant JSON API
> endpoint, and decoding the returned JSON into a JSON Object (right now the
> top-level data types are all Objects).
> 
> Refactor this boilerplate into a new method, get_api_json_object().
> 
> Signed-off-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> 
> ---

While you're refactoring, can you get the Jenkins code to use the client created
in main?  This is doubly important now we have proxy support
Andrew Donnellan June 16, 2016, 4:10 a.m. UTC | #2 
On 16/06/16 11:35, Russell Currey wrote:
> While you're refactoring, can you get the Jenkins code to use the client created
> in main?  This is doubly important now we have proxy support

That's fair - I'll fold that into a V2.
diff mbox

Patch

diff --git a/src/jenkins.rs b/src/jenkins.rs
index 8ce9c3c..fb10829 100644
--- a/src/jenkins.rs
+++ b/src/jenkins.rs
@@ -74,34 +74,27 @@  pub enum JenkinsBuildStatus {
 }
 
 impl<'a> JenkinsBackend<'a> {
-    pub fn get_build_url(&self, build_queue_entry: &str) -> Option<String> {
-        let client = Client::new(); // TODO
-        let url = format!("{}api/json", build_queue_entry);
-
-        let mut resp = client.get(&url).send().expect("HTTP request error"); // TODO don't panic here
+    fn get_api_json_object(&self, base_url: &str) -> rustc_serialize::json::Object {
+        // TODO: Don't panic on failure, fail more gracefully
+        let client = Client::new(); // TODO: Can we reduce the number of Clients we create?
+        let url = format!("{}api/json", base_url);
+        let mut resp = client.get(&url).send().expect("HTTP request error");
         let mut result_str = String::new();
         resp.read_to_string(&mut result_str)
             .unwrap_or_else(|err| panic!("Couldn't read from server: {}", err));
-        let json = Json::from_str(&result_str).unwrap();
-        let obj = json.as_object().unwrap();
-
-        match obj.get("executable") {
+        let json = Json::from_str(&result_str).unwrap_or_else(|err| panic!("Couldn't parse JSON from Jenkins: {}", err));
+        json.as_object().unwrap().clone()
+    }
+    
+    pub fn get_build_url(&self, build_queue_entry: &str) -> Option<String> {
+        match self.get_api_json_object(build_queue_entry).get("executable") {
             Some(exec) => Some(exec.as_object().unwrap().get("url").unwrap().as_string().unwrap().to_string()),
             None => None
         }
     }
 
     pub fn get_build_status(&self, build_url: &str) -> JenkinsBuildStatus {
-        let client = Client::new();
-        let url = format!("{}api/json", build_url);
-        let mut resp = client.get(&url).send().expect("HTTP request error");
-        let mut result_str = String::new();
-        resp.read_to_string(&mut result_str)
-            .unwrap_or_else(|err| panic!("Couldn't read from server: {}", err));
-        let json = Json::from_str(&result_str)
-            .unwrap_or_else(|err| panic!("Couldn't decode JSON: {}", err));
-
-        match json.as_object().unwrap().get("building").unwrap().as_boolean().unwrap() {
+        match self.get_api_json_object(build_url).get("building").unwrap().as_boolean().unwrap() {
             true => JenkinsBuildStatus::Running,
             false => JenkinsBuildStatus::Done,
         }