remove Mutexes and use RwLock

523be883 · Pierre Jarriges · 3a8686ef · 523be883 · 523be883 · 523be883
Commit 523be883 authored 2 years ago by Pierre Jarriges
--- a/src/main.rs
+++ b/src/main.rs
@@ -25,7 +25,8 @@ async fn main() -> std::io::Result<()> {

    let website = WebSiteBuilder::load(&app_state.config)
        .with_static_files_manager(StaticFilesManager::new(&app_state).unwrap().build())
-        .build();
+        .build()
+        .unwrap();

    let host = app_state.config.host.clone();
    let port = app_state.config.port;
@@ -33,9 +34,9 @@ async fn main() -> std::io::Result<()> {

    let srv_conf = tls_config(&app_state.config);
    let dir = website.static_files_manager.dir.clone();
-    let mut_app_state = std::sync::Mutex::new(app_state);
+    let mut_app_state = std::sync::RwLock::new(app_state);
    let app_state = web::Data::new(mut_app_state);
-    let mut_website = web::Data::new(std::sync::Mutex::new(website));
+    let mut_website = web::Data::new(std::sync::RwLock::new(website));

    HttpServer::new(move || {
        App::new()

--- a/src/middleware/authentication.rs
+++ b/src/middleware/authentication.rs
@@ -2,10 +2,11 @@ use crate::{app::AdminAuthToken, AppState};
 use actix_web::{
    body::{EitherBody, MessageBody},
    dev::{forward_ready, Service, ServiceRequest, ServiceResponse, Transform},
-    Error,
+    web, Error, HttpResponse,
 };
 use futures::prelude::future::LocalBoxFuture;
 use std::future::{ready, Ready};
+use std::sync::RwLock;

 #[derive(Clone)]
 pub struct AuthService;
@@ -53,10 +54,10 @@ where
    fn call(&self, req: ServiceRequest) -> Self::Future {
        let token = {
            let app_state = req
-                .app_data::<actix_web::web::Data<std::sync::Mutex<AppState>>>()
+                .app_data::<web::Data<RwLock<AppState>>>()
                .expect("Failed to extract AppState from ServiceRequest")
-                .lock()
-                .expect("Failed to lock AppState Mutex");
+                .read()
+                .unwrap();
            app_state.admin_auth_token.clone()
        };

@@ -66,7 +67,7 @@ where
            let mut req = req;
            if let false = authenticate(&mut req, &token).await {
                return Ok(req.into_response(
-                    actix_web::HttpResponse::Unauthorized()
+                    HttpResponse::Unauthorized()
                        .body("<html><body>Error 401 - Unauthorized - Please go to <a href='/admin/login'>login page</a>.</body></html>") // TODO a proper 401 view ?
                        .map_into_right_body(),
                ));

--- a/src/service/admin.rs
+++ b/src/service/admin.rs
@@ -5,6 +5,7 @@ use actix_web::{
    web::{Data, Form},
    HttpRequest, HttpResponse, Responder,
 };
+use std::sync::RwLock;

 #[get("/workspace")]
 async fn admin_workspace() -> impl Responder {
@@ -21,11 +22,13 @@ struct Credentials {
 #[post("/login")]
 async fn admin_authenticate(
    credentials: Form<Credentials>,
-    app_state: Data<std::sync::Mutex<AppState>>,
+    app_state: Data<RwLock<AppState>>,
    req: HttpRequest,
 ) -> impl Responder {
    let (admin_username, admin_pwd, cookie_name) = {
-        let app_state = app_state.lock().unwrap();
+        let app_state = app_state
+            .read()
+            .expect("Couldn't read to RwLock<AppState>.");
        (
            app_state.config.admin_username.to_owned(),
            app_state.config.admin_pwd.to_owned(),
@@ -35,7 +38,9 @@ async fn admin_authenticate(

    if admin_username.eq(&credentials.username) && admin_pwd.eq(&credentials.password) {
        let cookie_value = {
-            let mut app_state = app_state.lock().unwrap();
+            let mut app_state = app_state
+                .write()
+                .expect("Couldn't acquire write lock for RwLock<AppState>");
            app_state.admin_auth_token.generate();
            app_state
                .admin_auth_token

--- a/src/service/files.rs
+++ b/src/service/files.rs
@@ -11,12 +11,20 @@ use futures::StreamExt;
 use std::{
    fs::{remove_file, File},
    io::Write,
+    sync::RwLock,
 };

 #[get("/favicon.ico")]
-pub async fn favicon(website: web::Data<std::sync::Mutex<WebSite>>) -> impl Responder {
-    let static_files_manager = &website.lock().unwrap().static_files_manager;
-    NamedFile::open(static_files_manager.dir.join("default").join("favicon.ico"))
+pub async fn favicon(website: web::Data<RwLock<WebSite>>) -> impl Responder {
+    NamedFile::open(
+        &website
+            .read()
+            .unwrap()
+            .static_files_manager
+            .dir
+            .join("default")
+            .join("favicon.ico"),
+    )
 }

 fn upload_data_from_multipart_field(
@@ -68,12 +76,11 @@ async fn write_uploaded_file(
 }

 #[post("/post-files")]
-pub async fn post_files(
-    website: Data<std::sync::Mutex<WebSite>>,
-    mut payload: Multipart,
-) -> impl Responder {
+pub async fn post_files(website: Data<RwLock<WebSite>>, mut payload: Multipart) -> impl Responder {
    let mut uploaded_filepathes = Vec::new();
-    let mut website = website.lock().unwrap();
+    let mut website = website
+        .write()
+        .expect("Couldn't acquire write lock for RwLock<WebSite");

    while let Some(item) = payload.next().await {
        match item {
@@ -107,22 +114,18 @@ pub async fn post_files(
 }

 #[get("/static-files-index")]
-async fn get_static_files_index(website: Data<std::sync::Mutex<WebSite>>) -> impl Responder {
-    HttpResponse::Ok().json(
-        website
-            .lock()
-            .expect("Couldn't lock website")
-            .static_files_manager
-            .get_index(),
-    )
+async fn get_static_files_index(website: Data<RwLock<WebSite>>) -> impl Responder {
+    HttpResponse::Ok().json(website.read().unwrap().static_files_manager.get_index())
 }

 #[delete("/delete-file/{category}/{filename}")]
 async fn delete_static_file(
-    website: Data<std::sync::Mutex<WebSite>>,
+    website: Data<RwLock<WebSite>>,
    fileinfo: Path<(String, String)>,
 ) -> impl Responder {
-    let mut website = website.lock().unwrap();
+    let mut website = website
+        .write()
+        .expect("Couldn't acquire write lock for RwLock<WebSite");
    let (cat, fname) = fileinfo.into_inner();
    let fpath = std::path::PathBuf::from(cat).join(fname);


--- a/src/website/page.rs
+++ b/src/website/page.rs
@@ -192,35 +192,39 @@ impl Page {
    pub fn build(
        &mut self,
        templates: &Vec<PageTemplate>,
-        from_url: PathBuf,
+        parent_scope: PathBuf,
        static_files_manager: &StaticFilesManager,
-    ) {
-        self.build_with_template(
-            templates
-                .iter()
-                .find(|t| t.name == self.template_name)
-                .expect("Page template not found")
-                .clone(),
-        );
-
+    ) -> Result<(), String> {
+        self.build_with_template(self.template(templates));
        self.build_html();
+        let url = self.url(parent_scope);

-        let url = from_url.join(&self.metadata.url_slug);
-
-        static_files_manager.write_html_page(&url, self).unwrap();
+        if let Err(err) = self.write_to_static_file(&url, static_files_manager) {
+            return Err(format!(
+                "Error writing html static file for page {} - {}",
+                self.metadata.title, err
+            ));
+        }

        for p in self.sub_pages.iter_mut() {
-            p.build(templates, url.clone(), static_files_manager);
+            if let Err(err) = p.build(templates, url.clone(), static_files_manager) {
+                return Err(format!(
+                    "Error building page {} - {}",
+                    p.metadata.title, err
+                ));
+            }
        }
+
+        Ok(())
    }

-    pub fn build_html(&mut self) {
+    fn build_html(&mut self) {
        self.html = HtmlDoc::from_page(self);
    }

-    pub fn build_with_template(&mut self, template: PageTemplate) {
+    fn build_with_template(&mut self, template: &PageTemplate) {
        let mut complete_body = template.clone();
-        self.template = Some(template);
+        self.template = Some(template.clone());

        PageTemplate::get_page_body_placeholder(&mut complete_body.contents)
            .expect("Couldn't find page body container in template")
@@ -229,6 +233,24 @@ impl Page {
        self.body.0 = complete_body.contents;
    }

+    fn url(&self, parent_scope: PathBuf) -> PathBuf {
+        parent_scope.join(&self.metadata.url_slug)
+    }
+
+    fn template<'a>(&self, templates: &'a Vec<PageTemplate>) -> &'a PageTemplate {
+        templates
+            .iter()
+            .find(|t| t.name == self.template_name)
+            .expect("Page template not found")
+    }
+    fn write_to_static_file(
+        &self,
+        url: &PathBuf,
+        static_files_manager: &StaticFilesManager,
+    ) -> std::io::Result<()> {
+        static_files_manager.write_html_page(&url, self)
+    }
+
    pub fn to_map(&self) -> HashMap<String, String> {
        let mut map = HashMap::new();
        map.insert("title".to_string(), self.metadata.title.to_owned());
@@ -402,7 +424,7 @@ mod test_pages {
    #[test]
    fn build_body_with_template() {
        let mut page = test_page();
-        page.build_with_template(test_page_template());
+        page.build_with_template(&test_page_template());
        assert_eq!(
            page.body.to_string(),
            "<div id=\"test-template\"><nav>NAV</nav><div id=\"page-body\"><span>TEST</span></div><footer>FOOTER</footer></div>"

--- a/src/website/website.rs
+++ b/src/website/website.rs
@@ -53,12 +53,15 @@ impl WebSiteBuilder {
 }

 impl WebSite {
-    pub fn build(&mut self) -> Self {
-        self.root_page.build(
+    pub fn build(&mut self) -> Result<Self, String> {
+        if let Err(err) = self.root_page.build(
            &self.templates,
            PathBuf::from("/"),
            &self.static_files_manager,
-        );
-        self.clone()
+        ) {
+            return Err(err);
+        };
+
+        Ok(self.clone())
    }
 }