From 523be8831e30d9825a94134a1ea8148bd363a6ad Mon Sep 17 00:00:00 2001
From: Pierre Jarriges <pierre.jarriges@tutanota.com>
Date: Thu, 15 Sep 2022 09:16:48 +0200
Subject: [PATCH] remove Mutexes and use RwLock
---
src/main.rs | 7 ++--
src/middleware/authentication.rs | 11 +++---
src/service/admin.rs | 11 ++++--
src/service/files.rs | 39 +++++++++++----------
src/website/page.rs | 58 ++++++++++++++++++++++----------
src/website/website.rs | 11 +++---
6 files changed, 86 insertions(+), 51 deletions(-)
diff --git a/src/main.rs b/src/main.rs
index 00a89e6..54299c1 100644
--- 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()
diff --git a/src/middleware/authentication.rs b/src/middleware/authentication.rs
index 39008a9..faf9588 100644
--- 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(),
));
diff --git a/src/service/admin.rs b/src/service/admin.rs
index 0366a0c..6061c47 100644
--- 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
diff --git a/src/service/files.rs b/src/service/files.rs
index b84f024..e04a77d 100644
--- 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);
diff --git a/src/website/page.rs b/src/website/page.rs
index 92df3d3..c618440 100644
--- 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>"
diff --git a/src/website/website.rs b/src/website/website.rs
index d3a17dd..c5ee7d8 100644
--- 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())
}
}
--
GitLab