Merge branch 'no-copy-templatestate'

CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Internal Changes
+
+- Entirely rewrite `TemplateState` to use references instead of cloning
+  - Also remove askama_axum as it is easier to reimplement locally by rendering
+    template to String manually
+  - Also includes a major rewrite of how ErrorResponse works
+- Make `CurrentNavigation` type used in `TemplateState` be `Copy` since it is a
+  simple value enum
+- Remove unnecessary lifetime parameter from ExpiryUnit since they are always
+  `'static`
+
 ## [v0.2.11] - 2024-09-22
 
 ### Bugfixes

bobashare-web/Cargo.toml

@@ -13,8 +13,8 @@ default-run = "bobashare-web"
 
 [dependencies]
 anyhow = "1.0.65"
-askama = { version = "0.12.0", features = ["with-axum"] }
-askama_axum = "0.4.0"
+#askama = { version = "0.12.0", features = ["with-axum"] }
+askama = "0.12.0"
 axum = { version = "0.7.4", features = ["multipart"] }
 axum-extra = { version = "0.9.2", features = ["typed-header"] }
 bobashare = { path = "../bobashare" }

bobashare-web/src/main.rs

@@ -10,7 +10,7 @@ use axum::{self, routing::get, Router};
 use bobashare::storage::file::FileBackend;
 use bobashare_web::{
     api, static_routes, str_to_duration,
-    views::{self, ErrorResponse, ErrorTemplate},
+    views::{self, ErrorResponse, ErrorTemplate, TemplateState},
     AppState,
 };
 use chrono::TimeDelta;
@@ -167,17 +167,23 @@ async fn main() -> anyhow::Result<()> {
         "generated state from config"
     );
 
-    let state2 = state.clone();
     let app = Router::new()
         .nest("/api", api::router())
         .merge(views::router())
         .nest_service("/static", get(static_routes::handler))
-        .fallback(|| async {
-            ErrorResponse(ErrorTemplate {
-                code: StatusCode::NOT_FOUND,
-                message: "no route for the requested URL was found".into(),
-                state: state2.into(),
-            })
+        .fallback({
+            let state = Arc::clone(&state);
+            move || {
+                let state = Arc::clone(&state);
+                async move {
+                    let tmpl_state = TemplateState::from(&*state);
+                    ErrorResponse::from(ErrorTemplate {
+                        code: StatusCode::NOT_FOUND,
+                        message: "no route for the requested URL was found".into(),
+                        state: tmpl_state,
+                    })
+                }
+            }
         })
         .layer(
             ServiceBuilder::new()

bobashare-web/src/views/display.rs

@@ -23,7 +23,7 @@ use tokio_util::io::ReaderStream;
 use tracing::{event, instrument, Level};
 use url::Url;
 
-use super::{filters, prelude::*, ErrorResponse, ErrorTemplate, TemplateState};
+use super::{filters, prelude::*, render_template, ErrorResponse, ErrorTemplate, TemplateState};
 use crate::{AppState, CLASS_STYLE, MARKDOWN_OPTIONS};
 
 /// Errors when trying to view/download an upload
@@ -66,8 +66,8 @@ async fn open_upload<S: AsRef<str>>(
 
 #[derive(Template)]
 #[template(path = "display.html.jinja")]
-pub struct DisplayTemplate {
-    pub state: TemplateState,
+pub struct DisplayTemplate<'s> {
+    pub state: TemplateState<'s>,
     pub id: String,
     pub filename: String,
     pub expiry_date: Option<DateTime<Utc>>,
@@ -104,14 +104,15 @@ pub async fn display(
     State(state): State<Arc<AppState>>,
     Path(id): Path<String>,
 ) -> Result<impl IntoResponse, ErrorResponse> {
+    let tmpl_state = TemplateState::from(&*state);
     let mut upload = open_upload(&state, id).await.map_err(|e| match e {
         ViewUploadError::NotFound => ErrorTemplate {
-            state: state.clone().into(),
+            state: tmpl_state.clone(),
             code: StatusCode::NOT_FOUND,
             message: e.to_string(),
         },
         ViewUploadError::InternalServer(_) => ErrorTemplate {
-            state: state.clone().into(),
+            state: tmpl_state.clone(),
             code: StatusCode::INTERNAL_SERVER_ERROR,
             message: e.to_string(),
         },
@@ -121,7 +122,7 @@ pub async fn display(
         .metadata()
         .await
         .map_err(|e| ErrorTemplate {
-            state: state.clone().into(),
+            state: tmpl_state.clone(),
             code: StatusCode::INTERNAL_SERVER_ERROR,
             message: format!("error reading file size: {e}"),
         })?
@@ -148,7 +149,7 @@ pub async fn display(
                         .read_to_string(&mut contents)
                         .await
                         .map_err(|e| ErrorTemplate {
-                            state: state.clone().into(),
+                            state: tmpl_state.clone(),
                             code: StatusCode::INTERNAL_SERVER_ERROR,
                             message: format!("error reading file contents: {e}"),
                         })?;
@@ -168,7 +169,7 @@ pub async fn display(
                             generator
                                 .parse_html_for_line_which_includes_newline(line)
                                 .map_err(|e| ErrorTemplate {
-                                    state: state.clone().into(),
+                                    state: tmpl_state.clone(),
                                     code: StatusCode::INTERNAL_SERVER_ERROR,
                                     message: format!("error highlighting file contents: {e}"),
                                 })?;
@@ -201,7 +202,7 @@ pub async fn display(
                                         generator
                                             .parse_html_for_line_which_includes_newline(t)
                                             .map_err(|e| ErrorTemplate {
-                                                state: state.clone().into(),
+                                                state: tmpl_state.clone(),
                                                 code: StatusCode::INTERNAL_SERVER_ERROR,
                                                 message: format!(
                                                     "error highlighting markdown fenced code block: {e}",
@@ -242,7 +243,7 @@ pub async fn display(
     let raw_url = state.raw_url.join(&upload.metadata.id).unwrap();
     let mut download_url = raw_url.clone();
     download_url.set_query(Some("download"));
-    Ok(DisplayTemplate {
+    render_template(DisplayTemplate {
         raw_url,
         download_url,
         id: upload.metadata.id,
@@ -252,7 +253,7 @@ pub async fn display(
         size,
         mimetype: upload.metadata.mimetype,
         contents,
-        state: state.into(),
+        state: tmpl_state,
     })
 }
 
@@ -274,14 +275,15 @@ pub async fn raw(
     Path(id): Path<String>,
     Query(RawParams { download }): Query<RawParams>,
 ) -> Result<impl IntoResponse, ErrorResponse> {
+    let tmpl_state = TemplateState::from(&*state);
     let upload = open_upload(&state, id).await.map_err(|e| match e {
         ViewUploadError::NotFound => ErrorTemplate {
-            state: state.clone().into(),
+            state: tmpl_state.clone(),
             code: StatusCode::NOT_FOUND,
             message: e.to_string(),
         },
         ViewUploadError::InternalServer(_) => ErrorTemplate {
-            state: state.clone().into(),
+            state: tmpl_state.clone(),
             code: StatusCode::INTERNAL_SERVER_ERROR,
             message: e.to_string(),
         },
@@ -292,7 +294,7 @@ pub async fn raw(
         .metadata()
         .await
         .map_err(|e| ErrorTemplate {
-            state: state.clone().into(),
+            state: tmpl_state.clone(),
             code: StatusCode::INTERNAL_SERVER_ERROR,
             message: format!("error reading file size: {e}"),
         })?

bobashare-web/src/views/mod.rs

@@ -3,8 +3,12 @@
 use std::sync::Arc;
 
 use askama::Template;
-use askama_axum::IntoResponse;
-use axum::{routing::get, Router};
+use axum::{
+    http,
+    response::{IntoResponse, Response},
+    routing::get,
+    Router,
+};
 use chrono::Duration;
 use hyper::StatusCode;
 use tracing::{event, Level};
@@ -20,37 +24,34 @@ mod prelude {
     pub use super::CurrentNavigation;
 }
 
+// 's is for &AppState
+// TODO: should this be Copy
 #[derive(Debug, Clone)]
-pub struct TemplateState {
+pub struct TemplateState<'s> {
     version: &'static str,
-    base_url: Url,
+    base_url: &'s Url,
     max_file_size: u64,
     max_expiry: Option<Duration>,
-    extra_footer_text: Option<String>,
+    extra_footer_text: Option<&'s str>,
 
     // None if the current page is not a navbar item
     current_navigation: Option<CurrentNavigation>,
 }
-impl From<&AppState> for TemplateState {
-    fn from(state: &AppState) -> Self {
+impl<'s> From<&'s AppState> for TemplateState<'s> {
+    fn from(state: &'s AppState) -> Self {
         Self {
             version: env!("CARGO_PKG_VERSION"),
-            base_url: state.base_url.clone(),
+            base_url: &state.base_url,
             max_file_size: state.max_file_size,
             max_expiry: state.max_expiry,
-            extra_footer_text: state.extra_footer_text.clone(),
+            extra_footer_text: state.extra_footer_text.as_deref(),
             current_navigation: None, // will be set to Some in individual handlers
         }
     }
 }
-impl From<Arc<AppState>> for TemplateState {
-    fn from(state: Arc<AppState>) -> Self {
-        Self::from(&*state)
-    }
-}
 
 // which page is current navigated to, for navbar formatting
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, Copy)]
 #[non_exhaustive]
 pub enum CurrentNavigation {
     Upload,
@@ -59,34 +60,83 @@ pub enum CurrentNavigation {
 
 #[derive(Template)]
 #[template(path = "error.html.jinja")]
-pub struct ErrorTemplate {
-    pub state: TemplateState,
+pub struct ErrorTemplate<'s> {
+    pub state: TemplateState<'s>,
     pub code: StatusCode,
     pub message: String,
 }
 
-pub struct ErrorResponse(pub ErrorTemplate);
-impl From<ErrorTemplate> for ErrorResponse {
-    fn from(template: ErrorTemplate) -> Self {
-        Self(template)
+pub struct ErrorResponse(Response);
+impl From<ErrorTemplate<'_>> for ErrorResponse {
+    fn from(tmpl: ErrorTemplate) -> Self {
+        let error_msg = &tmpl.message;
+        match tmpl.render() {
+            Ok(s) => {
+                let status_num = tmpl.code.as_u16();
+                if tmpl.code.is_server_error() {
+                    event!(Level::ERROR, status = status_num, error_msg);
+                } else if tmpl.code.is_client_error() {
+                    event!(Level::WARN, status = status_num, error_msg);
+                } else {
+                    event!(Level::INFO, status = status_num, error_msg);
+                }
+                Self(
+                    (
+                        tmpl.code,
+                        [(
+                            http::header::CONTENT_TYPE,
+                            http::header::HeaderValue::from_static(ErrorTemplate::MIME_TYPE),
+                        )],
+                        s,
+                    )
+                        .into_response(),
+                )
+            }
+            Err(e) => {
+                let status = tmpl.code.as_u16();
+                event!(Level::ERROR, status, error_msg, render_error = ?e, "error rendering error page template, so HTTP 500 returned:");
+                Self(
+                    (
+                        StatusCode::INTERNAL_SERVER_ERROR,
+                        format!("internal error rendering error page template: {:?}", e),
+                    )
+                        .into_response(),
+                )
+            }
+        }
+    }
+}
+impl From<askama::Error> for ErrorResponse {
+    fn from(err: askama::Error) -> Self {
+        event!(Level::ERROR, render_error = ?err, "error rendering template");
+        Self(
+            (
+                StatusCode::INTERNAL_SERVER_ERROR,
+                format!("internal error rendering template: {:?}", err),
+            )
+                .into_response(),
+        )
     }
 }
 impl IntoResponse for ErrorResponse {
-    fn into_response(self) -> askama_axum::Response {
-        let code = self.0.code;
-        let error_msg = &self.0.message;
-        if code.is_server_error() {
-            event!(Level::ERROR, status = code.as_u16(), error = error_msg);
-        } else if code.is_client_error() {
-            event!(Level::WARN, status = code.as_u16(), error = error_msg);
-        } else {
-            event!(Level::INFO, status = code.as_u16(), error = error_msg);
-        }
-
-        (self.0.code, self.0).into_response()
+    fn into_response(self) -> axum::response::Response {
+        self.0
     }
 }
 
+pub(crate) fn render_template<T: askama::Template>(tmpl: T) -> Result<Response, ErrorResponse> {
+    let rendered = tmpl.render()?;
+    Ok((
+        StatusCode::OK,
+        [(
+            http::header::CONTENT_TYPE,
+            http::header::HeaderValue::from_static(T::MIME_TYPE),
+        )],
+        rendered,
+    )
+        .into_response())
+}
+
 pub fn router() -> Router<Arc<AppState>> {
     Router::new()
         .route("/", get(upload::upload))