From d4c3ec39c0767d4cb349d14f81c27e7cf6109dd3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gon=C3=A7alo=20Val=C3=A9rio?= <gon@ovalerio.net>
Date: Tue, 3 Sep 2019 11:57:54 +0100
Subject: [PATCH] request validation uses results instead of just returning
 true or false

---
 src/controllers.rs | 12 ++++++++----
 src/main.rs        | 14 ++++++++------
 src/utils.rs       | 45 ++++++++++++++++++++++++++++++++++-----------
 3 files changed, 50 insertions(+), 21 deletions(-)

diff --git a/src/controllers.rs b/src/controllers.rs
index 7bef152..35316e8 100644
--- a/src/controllers.rs
+++ b/src/controllers.rs
@@ -26,10 +26,14 @@ pub fn hub(state: web::Data<AppState>, _req: HttpRequest, params: String) -> Htt
         parameters.insert(key.to_string(), value.to_string());
     }
 
-    if !validate_parsed_data(&parameters) {
-        return HttpResponse::Ok()
-            .status(http::StatusCode::BAD_REQUEST)
-            .finish();
+    match validate_parsed_data(&parameters) {
+        Ok(_) => debug!(log, "Valid request."),
+        Err(reason) => {
+            return HttpResponse::Ok()
+                .status(http::StatusCode::BAD_REQUEST)
+                .content_type("text/plain")
+                .body(reason)
+        }
     }
 
     handle_subscription(db, &parameters);
diff --git a/src/main.rs b/src/main.rs
index 96562f0..b46e4f7 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -9,7 +9,7 @@ extern crate slog;
 extern crate slog_async;
 extern crate slog_term;
 extern crate url;
-use actix::{System};
+use actix::System;
 use actix_web::{web, App, HttpServer};
 use clap::Arg;
 use controllers::{hub, index};
@@ -24,8 +24,6 @@ mod schema;
 mod utils;
 
 fn main() {
-    let log = setup_logging();
-    info!(log, "Launching hub");
     let matches = clap::App::new("Rusty Hub")
         .version("0.1.0")
         .author("Gonçalo Valério <gon@ovalerio.net>")
@@ -35,23 +33,27 @@ fn main() {
                 .short("c")
                 .long("config")
                 .value_name("FILE")
-                .help("Sets a custom config file")
+                .help("Set a custom config file")
                 .takes_value(true),
         )
         .get_matches();
 
+    let log = setup_logging();
+    info!(log, "Launching hub");
+
     let address = "127.0.0.1";
     let port = "8888";
+    let storage = "local.db";
 
     info!(log, "Loading configuration");
     let config = matches.value_of("config").unwrap_or("");
     if !config.is_empty() {
-        println!("Not implemented");
+        error!(log, "Configuration not implemented yet");
         return;
     }
 
     let sys = System::new("rusty-hub");
-    let manager = ConnectionManager::<SqliteConnection>::new("local.db");
+    let manager = ConnectionManager::<SqliteConnection>::new(storage);
     let pool = r2d2::Pool::builder()
         .build(manager)
         .expect("Failed to create pool.");
diff --git a/src/utils.rs b/src/utils.rs
index a577439..f21f7c9 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -19,39 +19,62 @@ pub fn setup_logging() -> slog::Logger {
     slog::Logger::root(drain, o!())
 }
 
-pub fn validate_parsed_data(parameters: &HashMap<String,String>) -> bool {
+pub fn validate_parsed_data(parameters: &HashMap<String, String>) -> Result<(), String> {
     let callback;
     let mode;
     let topic;
 
     match parameters.get("hub.callback") {
         Some(value) => callback = value,
-        None => return false,
+        None => return Err("No hub.callback specified".to_owned()),
     };
 
     match parameters.get("hub.mode") {
         Some(value) => mode = value,
-        None => return false,
+        None => return Err("No hub.mode specified".to_owned()),
     };
 
     match parameters.get("hub.topic") {
         Some(value) => topic = value,
-        None => return false,
+        None => return Err("No hub.topicspecified".to_owned()),
     };
 
     if mode != &"subscribe" && mode != &"unsubscribe" {
-        debug!(setup_logging(), "Invalid Method: {}", mode);
-        return false;
+        return Err(format!("Invalid Method: {}", mode));
     }
 
     match Url::parse(callback) {
         Ok(value) => debug!(setup_logging(), "Valid Callback: {}", value),
-        Err(_) => return false,
-    }
+        Err(_) => return Err("hub.callback is not a valid URL".to_owned()),
+    };
 
     match Url::parse(topic) {
         Ok(value) => debug!(setup_logging(), "Valid Topic: {}", value),
-        Err(_) => return false,
-    }
-    true
+        Err(_) => return Err("hub.topic is not a valid URL".to_owned()),
+    };
+    Ok(())
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::collections::HashMap;
+
+    #[test]
+    fn test_validate_parsed_data_is_valid() {
+        let mut params = HashMap::new();
+        params.insert("hub.callback".to_string(), "http://example.com".to_string());
+        params.insert("hub.topic".to_string(), "http://example2.com".to_string());
+        params.insert("hub.mode".to_string(), "subscribe".to_string());
+
+        let result = validate_parsed_data(&params);
+        assert_eq!(result.is_ok(), true);
+    }
+
+    #[test]
+    fn test_validate_parsed_data_is_invalid() {
+        let params = HashMap::new();
+        let result = validate_parsed_data(&params);
+        assert_eq!(result.is_ok(), false);
+    }
 }