improved the error handling

src/api/error.rs

@@ -1,9 +1,36 @@
+use actix_web::{
+    http::{header::ContentType, StatusCode},
+    HttpResponse, ResponseError,
+};
 use derive_more::{Display, Error};
 
-impl actix_web::error::ResponseError for ReturnedError {}
-
 #[derive(Debug, Display, Error)]
-#[display("my error: {}", name)]
-pub struct ReturnedError {
-    pub name: String,
+pub enum ResponseMRDErrors {
+    #[display("Character {} not allowed in {}", character, parameter)]
+    CharacterNotAllowed { character: char, parameter: String },
+
+    #[display("container not found")]
+    ContainerNotFound,
+
+    #[display("error: {}", message)]
+    GenericError { message: String },
+}
+
+impl ResponseError for ResponseMRDErrors {
+    fn error_response(&self) -> HttpResponse {
+        HttpResponse::build(self.status_code())
+            .insert_header(ContentType::html())
+            .body(self.to_string())
+    }
+
+    fn status_code(&self) -> StatusCode {
+        match *self {
+            ResponseMRDErrors::CharacterNotAllowed {
+                character: _,
+                parameter: _,
+            } => StatusCode::BAD_REQUEST,
+            ResponseMRDErrors::ContainerNotFound => StatusCode::NOT_FOUND,
+            ResponseMRDErrors::GenericError { message: _ } => StatusCode::INTERNAL_SERVER_ERROR,
+        }
+    }
 }

src/api/routes.rs

@@ -1,7 +1,7 @@
 use std::net::Ipv4Addr;
 use std::str::FromStr;
 
-use crate::api::error::ReturnedError;
+use crate::api::error::ResponseMRDErrors;
 use crate::config::{
     docker::DockerConnectionConfig, mrproxy::MrproxyConnectionConfig, server::ServerConfig,
     CONFIG_PATH,
@@ -33,51 +33,48 @@ async fn create_container(
     params: web::Query<ConainerParams>,
 ) -> impl Responder {
     let ops = Options::new(params.memory.clone(), params.msg.clone());
-    controller
-        .create_container(
-            params.name.clone(),
-            params.ip.clone(),
-            params.image.clone(),
-            ops,
-        )
+    let name = params.name.clone();
+    for c in name.chars() {
+        if !c.is_ascii_alphanumeric() {
+            return Err(ResponseMRDErrors::CharacterNotAllowed {
+                character: c,
+                parameter: "domain name".to_string(),
+            });
+        }
+    }
+    match controller
+        .create_container(name, params.ip.clone(), params.image.clone(), ops)
         .await
+    {
+        Ok(result) => Ok(result),
+        Err(e) => Err(ResponseMRDErrors::GenericError {
+            message: e.to_string(),
+        }),
+    }
 }
 
 #[patch("/container/stop")]
 async fn stop_container(
     controller: web::Data<Controller>,
     params: web::Query<ConainerStopParams>,
-) -> Result<String, ReturnedError> {
-    match controller.stop_container(params.name.clone()).await {
-        Ok(response) => Ok(response),
-        Err(e) => Err(ReturnedError {
-            name: e.to_string(),
-        }),
-    }
+) -> impl Responder {
+    controller.stop_container(params.name.clone()).await
 }
 
 #[delete("/container")]
 async fn delete_container(
     controller: web::Data<Controller>,
     params: web::Query<ConainerStopParams>,
-) -> Result<String, ReturnedError> {
-    match controller.delete_container_and_unbind(&params.name).await {
-        Ok(response) => Ok(response),
-        Err(e) => Err(ReturnedError {
-            name: e.to_string(),
-        }),
-    }
+) -> impl Responder {
+    controller.delete_container_and_unbind(&params.name).await
 }
 
 #[get("/container/{container_name}/ip")]
 async fn get_ip(
     controller: web::Data<Controller>,
     container_name: web::Path<String>,
-) -> Result<String, ReturnedError> {
-    Ok(controller
-        .get_ip(container_name.into_inner())
-        .await
-        .unwrap())
+) -> impl Responder {
+    controller.get_ip(&container_name.into_inner()).await
 }
 
 pub async fn start() -> std::io::Result<()> {

src/controller/dns_controller.rs

@@ -0,0 +1,13 @@
+use crate::controller::Controller;
+
+impl Controller {
+    pub async fn add_domain_to_dns(&self, domain: &str) -> Result<(), Box<dyn std::error::Error>> {
+        self.dns_manager.add_domain(domain, self.pub_addr).await?;
+        Ok(())
+    }
+
+    pub async fn del_domain_to_dns(&self, domain: &str) -> Result<(), Box<dyn std::error::Error>> {
+        self.dns_manager.del_domain(domain).await?;
+        Ok(())
+    }
+}

src/controller/docker_controller.rs

@@ -0,0 +1,53 @@
+use std::error::Error;
+
+use crate::{
+    controller::Controller,
+    deploy::{self, container::Container, container_options::Options},
+};
+
+impl Controller {
+    pub async fn load_container(
+        &self,
+        docker_id: Option<String>,
+        domain: String,
+        ip: Option<String>,
+        image: String,
+        ops: Options,
+    ) -> Result<deploy::container::Container, bollard::errors::Error> {
+        deploy::container::Container::new(
+            self.driver.clone(),
+            docker_id,
+            domain,
+            ip,
+            image,
+            self.network.clone(),
+            ops,
+        )
+        .await
+    }
+
+    #[allow(dead_code)]
+    pub async fn is_started(&self) -> bool {
+        self.started
+    }
+
+    pub async fn prune_given_container(
+        &self,
+        container: Container,
+    ) -> Result<String, Box<dyn Error>> {
+        match container.remove(&self.driver).await {
+            Ok(_i) => Ok(container.get_id()),
+            Err(e) => Err(Box::new(e)),
+        }
+    }
+
+    pub async fn stop_given_container(
+        &self,
+        container: Container,
+    ) -> Result<String, bollard::errors::Error> {
+        match container.stop(&self.driver).await {
+            Ok(_i) => Ok(container.get_id()),
+            Err(e) => Err(e),
+        }
+    }
+}

src/controller/mixed_controller.rs

@@ -0,0 +1,71 @@
+use crate::{
+    controller::Controller,
+    database::instance::Instance,
+    deploy::{self, container_options::Options},
+};
+use log::error;
+
+impl Controller {
+    pub async fn load_container_and_bind(
+        &self,
+        docker_id: Option<String>,
+        domain: String,
+        ip: Option<String>,
+        image: String,
+        ops: Options,
+    ) -> Result<deploy::container::Container, Box<dyn std::error::Error>> {
+        match self
+            .load_container(docker_id, domain.clone(), ip.clone(), image, ops)
+            .await
+        {
+            Ok(container) => match self
+                .bind_container_in_proxy(
+                    &domain,
+                    &ip.unwrap_or(container.get_ip(&self.driver).await?),
+                )
+                .await
+            {
+                Ok(_) => {
+                    self.add_domain_to_dns(&domain).await?;
+                    Ok(container)
+                }
+                Err(e) => {
+                    error!("failed in the bind process with the proxy, deleting the container");
+                    self.delete_container_and_unbind(&domain).await?;
+                    return Err(e);
+                }
+            },
+            Err(e) => return Err(Box::new(e)),
+        }
+    }
+
+    pub async fn start_container_from_instance(&self, instance: Instance) {
+        let image = match self
+            .storage
+            .lock()
+            .unwrap()
+            .id_to_image(instance.image.clone())
+        {
+            Ok(i) => i,
+            Err(e) => {
+                error!("image not found: {}", e);
+                return;
+            }
+        };
+        match self
+            .load_container_and_bind(
+                Some(instance.docker_id.clone()),
+                instance.domain.clone(),
+                instance.ip.clone(),
+                image,
+                Options::new(None, None),
+            )
+            .await
+        {
+            Ok(c) => {
+                self.storage.lock().unwrap().loaded_instance(instance, c);
+            }
+            Err(e) => error!("{}", e),
+        }
+    }
+}

src/controller/mod.rs

@@ -1,10 +1,8 @@
+use crate::api::error::ResponseMRDErrors;
 use crate::config::mrproxy::MrproxyConnectionConfig;
 use crate::database::exposer::MemStorage;
-use crate::database::instance::Instance;
-use crate::deploy::container::Container;
+use crate::deploy;
 use crate::deploy::container_options::Options;
-use crate::{deploy, mcproxy_client};
-use bollard::errors::Error;
 use bollard::Docker;
 use dns_manager::{DnsManager, ServerZonesConnection};
 use log::{error, warn};
@@ -12,6 +10,11 @@ use std::error::Error as GenericError;
 use std::net::Ipv4Addr;
 use std::sync::Mutex;
 
+mod dns_controller;
+mod docker_controller;
+mod mixed_controller;
+mod mrproxy_controller;
+
 pub struct Controller {
     driver: Docker,
     pub_addr: Ipv4Addr,
@@ -50,133 +53,34 @@ impl Controller {
         ip: Option<String>,
         image: String,
         ops: Options,
-    ) -> String {
-        let is_stored = self.storage.lock().unwrap().search_instance(domain.clone());
+    ) -> Result<String, Box<dyn GenericError>> {
+        let is_stored = self.storage.lock().unwrap().search_instance(&domain);
         match is_stored {
             Some(c) => match c.docker_id {
-                Some(id) => id,
-                None => "Container without docker_id".to_string(),
+                Some(id) => Ok(id),
+                None => Ok("Container without docker_id".to_string()),
             },
             None => {
                 match self
-                    .load_container_and_bind(None, domain.clone(), ip.clone(), image.clone(), ops)
+                    .load_container_and_bind(None, domain, ip, image, ops)
                     .await
                 {
                     Ok(c) => {
-                        self.storage
-                            .try_lock()
-                            .unwrap()
-                            .new_instance(c.clone())
-                            .unwrap();
-                        c.get_id()
+                        let c_id = c.get_id();
+                        self.storage.try_lock().unwrap().new_instance(c)?;
+                        Ok(c_id)
                     }
-                    Err(e) => format!("error creating container: {}", e.to_string()),
+                    Err(e) => Err(e),
                 }
             }
         }
     }
 
-    async fn load_container_and_bind(
-        &self,
-        docker_id: Option<String>,
-        domain: String,
-        ip: Option<String>,
-        image: String,
-        ops: Options,
-    ) -> Result<deploy::container::Container, Box<dyn GenericError>> {
-        match self
-            .load_container(docker_id, domain.clone(), ip.clone(), image, ops)
-            .await
-        {
-            Ok(c) => match self.bind_container_in_proxy(&domain, ip, &c).await {
-                Ok(_) => {
-                    self.add_domain_to_dns(&domain).await?;
-                    Ok(c)
+    pub async fn stop_container(&self, domain: String) -> Result<String, ResponseMRDErrors> {
+        match self.storage.lock().unwrap().stop_instance(domain) {
+            Some(c) => format_error_bollard(self.stop_given_container(c).await),
+            None => Err(ResponseMRDErrors::ContainerNotFound),
         }
-                Err(e) => {
-                    error!("failed in the bind process with the proxy, deleting the container");
-                    self.delete_container_and_unbind(&domain).await?;
-                    return Err(e);
-                }
-            },
-            Err(e) => return Err(Box::new(e)),
-        }
-    }
-
-    async fn bind_container_in_proxy(
-        &self,
-        domain: &str,
-        ip: Option<String>,
-        container: &Container,
-    ) -> Result<(), Box<dyn GenericError>> {
-        let ip_final = ip.unwrap_or(container.get_ip(&self.driver).await?);
-        log::debug!("ip binded: {}", ip_final);
-        let mut mrcp_controller =
-            mcproxy_client::controller::Controller::new(&self.mrproxy_config)?;
-        mrcp_controller.insert_new_domain(&self.dns_manager.get_full_domain(domain), &ip_final)?;
-        Ok(())
-    }
-
-    async fn unbind_container_in_proxy(&self, domain: &str) -> Result<(), Box<dyn GenericError>> {
-        let mut mrcp_controller =
-            mcproxy_client::controller::Controller::new(&self.mrproxy_config)?;
-        mrcp_controller.remove_domain(&self.dns_manager.get_full_domain(domain))?;
-        Ok(())
-    }
-
-    pub async fn start_container_from_instance(&self, instance: Instance) {
-        let image = match self
-            .storage
-            .lock()
-            .unwrap()
-            .id_to_image(instance.image.clone())
-        {
-            Ok(i) => i,
-            Err(e) => {
-                error!("image not found: {}", e);
-                return;
-            }
-        };
-        match self
-            .load_container_and_bind(
-                Some(instance.docker_id.clone()),
-                instance.domain.clone(),
-                instance.ip.clone(),
-                image,
-                Options::new(None, None),
-            )
-            .await
-        {
-            Ok(c) => {
-                self.storage.lock().unwrap().loaded_instance(instance, c);
-            }
-            Err(e) => error!("{}", e),
-        }
-    }
-
-    async fn load_container(
-        &self,
-        docker_id: Option<String>,
-        domain: String,
-        ip: Option<String>,
-        image: String,
-        ops: Options,
-    ) -> Result<deploy::container::Container, bollard::errors::Error> {
-        deploy::container::Container::new(
-            self.driver.clone(),
-            docker_id,
-            domain,
-            ip,
-            image,
-            self.network.clone(),
-            ops,
-        )
-        .await
-    }
-
-    #[allow(dead_code)]
-    pub async fn is_started(&self) -> bool {
-        self.started
     }
 
     pub async fn load_all_instances(&self) -> bool {
@@ -194,37 +98,10 @@ impl Controller {
         return true;
     }
 
-    async fn stop_given_container(&self, container: Container) -> Result<String, Error> {
-        match container.stop(&self.driver).await {
-            Ok(_i) => Ok(container.get_id()),
-            Err(e) => Err(e),
-        }
-    }
-
-    async fn prune_given_container(
-        &self,
-        container: Container,
-    ) -> Result<String, Box<dyn GenericError>> {
-        match container.remove(&self.driver).await {
-            Ok(_i) => Ok(container.get_id()),
-            Err(e) => Err(Box::new(e)),
-        }
-    }
-
-    pub async fn stop_container(&self, domain: String) -> Result<String, Error> {
-        match self.storage.lock().unwrap().stop_instance(domain) {
-            Some(c) => self.stop_given_container(c).await,
-            None => Err(Error::DockerResponseServerError {
-                status_code: 404,
-                message: "container not found".to_string(),
-            }),
-        }
-    }
-
     pub async fn delete_container_and_unbind(
         &self,
         domain: &str,
-    ) -> Result<String, Box<dyn GenericError>> {
+    ) -> Result<String, ResponseMRDErrors> {
         match self.storage.lock().unwrap().remove_instance(&domain) {
             Some(data) => match data.1 {
                 Some(c) => {
@@ -236,34 +113,40 @@ impl Controller {
                     if r_del_domain.is_err() {
                         warn!("Error deleting domain {} from dns", domain);
                     }
-                    self.prune_given_container(c).await
+                    format_error(self.prune_given_container(c).await)
                 }
                 None => Ok(data.0.docker_id),
             },
-            None => Err(Box::new(Error::DockerResponseServerError {
-                status_code: 404,
-                message: "container not found".to_string(),
-            })),
+            None => Err(ResponseMRDErrors::ContainerNotFound),
         }
     }
 
-    pub async fn get_ip(&self, domain: String) -> Result<String, Error> {
-        let container = self
-            .storage
-            .lock()
-            .unwrap()
-            .search_instance(domain.clone())
-            .unwrap();
-        container.get_ip(&self.driver).await
+    pub async fn get_ip(&self, domain: &str) -> Result<String, ResponseMRDErrors> {
+        match self.storage.lock().unwrap().search_instance(domain) {
+            Some(container) => format_error_bollard(container.get_ip(&self.driver).await),
+            None => Err(ResponseMRDErrors::ContainerNotFound),
+        }
+    }
 }
 
-    pub async fn add_domain_to_dns(&self, domain: &str) -> Result<(), Box<dyn GenericError>> {
-        self.dns_manager.add_domain(domain, self.pub_addr).await?;
-        Ok(())
+fn format_error(
+    result: Result<String, Box<dyn GenericError>>,
+) -> Result<String, ResponseMRDErrors> {
+    match result {
+        Ok(r) => Ok(r),
+        Err(e) => Err(ResponseMRDErrors::GenericError {
+            message: e.to_string(),
+        }),
+    }
 }
 
-    pub async fn del_domain_to_dns(&self, domain: &str) -> Result<(), Box<dyn GenericError>> {
-        self.dns_manager.del_domain(domain).await?;
-        Ok(())
+fn format_error_bollard(
+    result: Result<String, bollard::errors::Error>,
+) -> Result<String, ResponseMRDErrors> {
+    match result {
+        Ok(r) => Ok(r),
+        Err(e) => Err(ResponseMRDErrors::GenericError {
+            message: e.to_string(),
+        }),
     }
 }

src/controller/mrproxy_controller.rs

@@ -0,0 +1,24 @@
+use crate::{controller::Controller, mcproxy_client};
+
+impl Controller {
+    pub async fn bind_container_in_proxy(
+        &self,
+        domain: &str,
+        ip: &str,
+    ) -> Result<(), Box<dyn std::error::Error>> {
+        let mut mrcp_controller =
+            mcproxy_client::controller::Controller::new(&self.mrproxy_config)?;
+        mrcp_controller.insert_new_domain(&self.dns_manager.get_full_domain(domain), ip)?;
+        Ok(())
+    }
+
+    pub async fn unbind_container_in_proxy(
+        &self,
+        domain: &str,
+    ) -> Result<(), Box<dyn std::error::Error>> {
+        let mut mrcp_controller =
+            mcproxy_client::controller::Controller::new(&self.mrproxy_config)?;
+        mrcp_controller.remove_domain(&self.dns_manager.get_full_domain(domain))?;
+        Ok(())
+    }
+}

src/database/exposer.rs

@@ -53,7 +53,7 @@ impl MemStorage {
         Ok(())
     }
 
-    pub fn search_instance(&self, name: String) -> Option<Container> {
+    pub fn search_instance(&self, name: &str) -> Option<Container> {
         for cont in &self.containers {
             if cont.0.domain == name {
                 return cont.1.clone();