Skip to content

Commit

Permalink
Use a separate trait for object safety (#579)
Browse files Browse the repository at this point in the history
This is relatively major change to the main trait's API. For context,
I always started from the concept of monomorphized traits, but later
several contributors asked about object safety. At that point I made
`Template` object-safe, and then even later added a `SizedTemplate`
to make some things easier for people who don't need object safety.

However, having object-safety in the primary trait is bad for
performance (a substantial number of calls into the virtual `Write`
trait is relatively slow), and I don't think those who don't need
object safety should pay for the cost of having it.

Additionally, I feel using associated consts for the extension and
size hint is more idiomatic than having accessor methods. I don't
know why I didn't use these from the start -- maybe associated
consts didn't exist yet, or I didn't yet know how/when to use them.
Askama is pretty old at this point...
  • Loading branch information
djc committed Dec 15, 2021
1 parent 228f6b2 commit 5cfef32
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 39 deletions.
77 changes: 69 additions & 8 deletions askama/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,26 +73,59 @@ use std::path::Path;
pub use askama_escape::{Html, Text};

/// Main `Template` trait; implementations are generally derived
///
/// If you need an object-safe template, use [`DynTemplate`].
pub trait Template {
/// Helper method which allocates a new `String` and renders into it
fn render(&self) -> Result<String> {
let mut buf = String::with_capacity(self.size_hint());
let mut buf = String::with_capacity(Self::SIZE_HINT);
self.render_into(&mut buf)?;
Ok(buf)
}

/// Renders the template to the given `writer` buffer
fn render_into(&self, writer: &mut (impl std::fmt::Write + ?Sized)) -> Result<()>;

/// The template's extension, if provided
const EXTENSION: Option<&'static str>;

/// Provides a conservative estimate of the expanded length of the rendered template
const SIZE_HINT: usize;
}

/// Object-safe wrapper trait around [`Template`] implementers
///
/// This trades reduced performance (mostly due to writing into `dyn Write`) for object safety.
pub trait DynTemplate {
/// Helper method which allocates a new `String` and renders into it
fn dyn_render(&self) -> Result<String>;

/// Renders the template to the given `writer` buffer
fn render_into(&self, writer: &mut dyn std::fmt::Write) -> Result<()>;
fn dyn_render_into(&self, writer: &mut dyn std::fmt::Write) -> Result<()>;

/// Helper function to inspect the template's extension
fn extension(&self) -> Option<&'static str>;
/// Provides an conservative estimate of the expanded length of the rendered template

/// Provides a conservative estimate of the expanded length of the rendered template
fn size_hint(&self) -> usize;
}

pub trait SizedTemplate {
/// Helper function to inspect the template's extension
fn extension() -> Option<&'static str>;
/// Provides an conservative estimate of the expanded length of the rendered template
fn size_hint() -> usize;
impl<T: Template> DynTemplate for T {
fn dyn_render(&self) -> Result<String> {
<Self as Template>::render(self)
}

fn dyn_render_into(&self, writer: &mut dyn std::fmt::Write) -> Result<()> {
<Self as Template>::render_into(self, writer)
}

fn extension(&self) -> Option<&'static str> {
Self::EXTENSION
}

fn size_hint(&self) -> usize {
Self::SIZE_HINT
}
}

pub use crate::shared::filters;
Expand Down Expand Up @@ -137,3 +170,31 @@ pub mod mime {
note = "file-level dependency tracking is handled automatically without build script"
)]
pub fn rerun_if_templates_changed() {}

#[cfg(test)]
mod tests {
use super::{DynTemplate, Template};

#[test]
fn dyn_template() {
struct Test;
impl Template for Test {
fn render_into(
&self,
writer: &mut (impl std::fmt::Write + ?Sized),
) -> askama_shared::Result<()> {
Ok(writer.write_str("test")?)
}

const EXTENSION: Option<&'static str> = Some("txt");

const SIZE_HINT: usize = 4;
}

fn render(t: &dyn DynTemplate) -> String {
t.dyn_render().unwrap()
}

assert_eq!(render(&Test), "test");
}
}
5 changes: 2 additions & 3 deletions askama_actix/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@ pub trait TemplateToResponse {

impl<T: askama::Template> TemplateToResponse for T {
fn to_response(&self) -> HttpResponse {
let mut buffer = BytesMut::with_capacity(self.size_hint());
let mut buffer = BytesMut::with_capacity(T::SIZE_HINT);
if self.render_into(&mut buffer).is_err() {
return ErrorInternalServerError("Template parsing error").error_response();
}

let ctype =
askama::mime::extension_to_mime_type(self.extension().unwrap_or("txt")).to_string();
let ctype = askama::mime::extension_to_mime_type(T::EXTENSION.unwrap_or("txt")).to_string();
HttpResponse::Ok()
.content_type(ctype.as_str())
.body(buffer.freeze())
Expand Down
22 changes: 5 additions & 17 deletions askama_shared/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
) -> Result<(), CompileError> {
self.write_header(buf, "::askama::Template", None)?;
buf.writeln(
"fn render_into(&self, writer: &mut dyn ::std::fmt::Write) -> \
"fn render_into(&self, writer: &mut (impl ::std::fmt::Write + ?Sized)) -> \
::askama::Result<()> {",
)?;

Expand Down Expand Up @@ -160,25 +160,13 @@ impl<'a, S: std::hash::BuildHasher> Generator<'a, S> {
buf.writeln("Ok(())")?;
buf.writeln("}")?;

buf.writeln("fn extension(&self) -> Option<&'static str> {")?;
buf.writeln("const EXTENSION: ::std::option::Option<&'static ::std::primitive::str> = ")?;
buf.writeln(&format!("{:?}", self.input.extension()))?;
buf.writeln("}")?;
buf.writeln(";")?;

buf.writeln("fn size_hint(&self) -> usize {")?;
buf.writeln("const SIZE_HINT: ::std::primitive::usize = ")?;
buf.writeln(&format!("{}", size_hint))?;
buf.writeln("}")?;

buf.writeln("}")?;

self.write_header(buf, "::askama::SizedTemplate", None)?;

buf.writeln("fn size_hint() -> usize {")?;
buf.writeln(&format!("{}", size_hint))?;
buf.writeln("}")?;

buf.writeln("fn extension() -> Option<&'static str> {")?;
buf.writeln(&format!("{:?}", self.input.extension()))?;
buf.writeln("}")?;
buf.writeln(";")?;

buf.writeln("}")?;
Ok(())
Expand Down
12 changes: 6 additions & 6 deletions testing/tests/ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ struct PathHtml;
fn test_path_ext_html() {
let t = PathHtml;
assert_eq!(t.render().unwrap(), "foo.html");
assert_eq!(t.extension(), Some("html"));
assert_eq!(PathHtml::EXTENSION, Some("html"));
}

#[derive(Template)]
Expand All @@ -19,7 +19,7 @@ struct PathJinja;
fn test_path_ext_jinja() {
let t = PathJinja;
assert_eq!(t.render().unwrap(), "foo.jinja");
assert_eq!(t.extension(), Some("jinja"));
assert_eq!(PathJinja::EXTENSION, Some("jinja"));
}

#[derive(Template)]
Expand All @@ -30,7 +30,7 @@ struct PathHtmlJinja;
fn test_path_ext_html_jinja() {
let t = PathHtmlJinja;
assert_eq!(t.render().unwrap(), "foo.html.jinja");
assert_eq!(t.extension(), Some("html"));
assert_eq!(PathHtmlJinja::EXTENSION, Some("html"));
}

#[derive(Template)]
Expand All @@ -41,7 +41,7 @@ struct PathHtmlAndExtTxt;
fn test_path_ext_html_and_ext_txt() {
let t = PathHtmlAndExtTxt;
assert_eq!(t.render().unwrap(), "foo.html");
assert_eq!(t.extension(), Some("txt"));
assert_eq!(PathHtmlAndExtTxt::EXTENSION, Some("txt"));
}

#[derive(Template)]
Expand All @@ -52,7 +52,7 @@ struct PathJinjaAndExtTxt;
fn test_path_ext_jinja_and_ext_txt() {
let t = PathJinjaAndExtTxt;
assert_eq!(t.render().unwrap(), "foo.jinja");
assert_eq!(t.extension(), Some("txt"));
assert_eq!(PathJinjaAndExtTxt::EXTENSION, Some("txt"));
}

#[derive(Template)]
Expand All @@ -63,5 +63,5 @@ struct PathHtmlJinjaAndExtTxt;
fn test_path_ext_html_jinja_and_ext_txt() {
let t = PathHtmlJinjaAndExtTxt;
assert_eq!(t.render().unwrap(), "foo.html.jinja");
assert_eq!(t.extension(), Some("txt"));
assert_eq!(PathHtmlJinjaAndExtTxt::EXTENSION, Some("txt"));
}
7 changes: 2 additions & 5 deletions testing/tests/simple.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![allow(clippy::blacklisted_name)]

use askama::{SizedTemplate, Template};
use askama::Template;

use std::collections::HashMap;

Expand All @@ -26,10 +26,7 @@ fn test_variables() {
Iñtërnâtiônàlizætiøn is important\n\
in vars too: Iñtërnâtiônàlizætiøn"
);
assert_eq!(
<VariablesTemplate as SizedTemplate>::extension(),
Some("html")
);
assert_eq!(VariablesTemplate::EXTENSION, Some("html"));
}

#[derive(Template)]
Expand Down

0 comments on commit 5cfef32

Please sign in to comment.