diff --git a/Cargo.lock b/Cargo.lock index 2f9bcd749..8d3c87b80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3727,6 +3727,7 @@ dependencies = [ "minijinja-contrib", "oauth2-types", "rand 0.8.5", + "rand_chacha 0.3.1", "serde", "serde_json", "serde_urlencoded", diff --git a/crates/cli/src/commands/server.rs b/crates/cli/src/commands/server.rs index 52465f077..cb723431b 100644 --- a/crates/cli/src/commands/server.rs +++ b/crates/cli/src/commands/server.rs @@ -166,6 +166,8 @@ impl Options { &url_builder, // Don't use strict mode in production yet false, + // Don't stabilise in production + false, ) .await?; shutdown.register_reloadable(&templates); diff --git a/crates/cli/src/commands/templates.rs b/crates/cli/src/commands/templates.rs index 8f1b0dd4e..1199e1f9a 100644 --- a/crates/cli/src/commands/templates.rs +++ b/crates/cli/src/commands/templates.rs @@ -8,6 +8,7 @@ use std::{fmt::Write, process::ExitCode}; use anyhow::{Context as _, bail}; use camino::Utf8PathBuf; +use chrono::DateTime; use clap::Parser; use figment::Figment; use mas_config::{ @@ -34,6 +35,12 @@ enum Subcommand { /// The directory must either not exist or be empty. #[arg(long = "out-dir")] out_dir: Option, + + /// Attempt to remove 'unstable' template input data such as asset + /// hashes, in order to make renders more reproducible between + /// versions. + #[arg(long = "stabilise")] + stabilise: bool, }, } @@ -41,7 +48,7 @@ impl Options { pub async fn run(self, figment: &Figment) -> anyhow::Result { use Subcommand as SC; match self.subcommand { - SC::Check { out_dir } => { + SC::Check { out_dir, stabilise } => { let _span = info_span!("cli.templates.check").entered(); let template_config = TemplatesConfig::extract_or_default(figment) @@ -59,9 +66,17 @@ impl Options { let captcha_config = CaptchaConfig::extract_or_default(figment) .map_err(anyhow::Error::from_boxed)?; - let clock = SystemClock::default(); - // XXX: we should disallow SeedableRng::from_entropy - let mut rng = rand_chacha::ChaChaRng::from_entropy(); + let now = if stabilise { + DateTime::from_timestamp_secs(1_446_823_992).unwrap() + } else { + SystemClock::default().now() + }; + let rng = if stabilise { + rand_chacha::ChaChaRng::from_seed([42; 32]) + } else { + // XXX: we should disallow SeedableRng::from_entropy + rand_chacha::ChaChaRng::from_entropy() + }; let url_builder = mas_router::UrlBuilder::new("https://example.com/".parse()?, None, None); let site_config = site_config_from_config( @@ -75,11 +90,13 @@ impl Options { let templates = templates_from_config( &template_config, &site_config, - &url_builder, // Use strict mode in template checks + &url_builder, + // Use strict mode in template checks true, + stabilise, ) .await?; - let all_renders = templates.check_render(clock.now(), &mut rng)?; + let all_renders = templates.check_render(now, &rng)?; if let Some(out_dir) = out_dir { // Save renders to disk. diff --git a/crates/cli/src/commands/worker.rs b/crates/cli/src/commands/worker.rs index a1eb0fcce..6dfcd27f1 100644 --- a/crates/cli/src/commands/worker.rs +++ b/crates/cli/src/commands/worker.rs @@ -58,6 +58,8 @@ impl Options { &url_builder, // Don't use strict mode on task workers for now false, + // Don't stabilise in production + false, ) .await?; diff --git a/crates/cli/src/util.rs b/crates/cli/src/util.rs index 4925d9866..cd69a8acf 100644 --- a/crates/cli/src/util.rs +++ b/crates/cli/src/util.rs @@ -233,11 +233,12 @@ pub async fn templates_from_config( site_config: &SiteConfig, url_builder: &UrlBuilder, strict: bool, + stabilise: bool, ) -> Result { Templates::load( config.path.clone(), url_builder.clone(), - config.assets_manifest.clone(), + (!stabilise).then(|| config.assets_manifest.clone()), config.translations_path.clone(), site_config.templates_branding(), site_config.templates_features(), diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index f1859f352..1f54b7022 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -172,7 +172,7 @@ impl TestState { let templates = Templates::load( workspace_root.join("templates"), url_builder.clone(), - workspace_root.join("frontend/dist/manifest.json"), + Some(workspace_root.join("frontend/dist/manifest.json")), workspace_root.join("translations"), site_config.templates_branding(), site_config.templates_features(), diff --git a/crates/templates/Cargo.toml b/crates/templates/Cargo.toml index ffe2dbf74..46ff80c74 100644 --- a/crates/templates/Cargo.toml +++ b/crates/templates/Cargo.toml @@ -25,6 +25,7 @@ http.workspace = true minijinja-contrib.workspace = true minijinja.workspace = true rand.workspace = true +rand_chacha.workspace = true serde_json.workspace = true serde_urlencoded.workspace = true serde.workspace = true @@ -42,3 +43,6 @@ mas-i18n.workspace = true mas-iana.workspace = true mas-router.workspace = true mas-spa.workspace = true + +[dev-dependencies] +rand_chacha.workspace = true diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 597683a03..4ed09c3e1 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -31,9 +31,10 @@ use mas_iana::jose::JsonWebSignatureAlg; use mas_router::{Account, GraphQL, PostAuthAction, UrlBuilder}; use oauth2_types::scope::{OPENID, Scope}; use rand::{ - Rng, + Rng, SeedableRng, distributions::{Alphanumeric, DistString}, }; +use rand_chacha::ChaCha8Rng; use serde::{Deserialize, Serialize, ser::SerializeStruct}; use ulid::Ulid; use url::Url; @@ -106,9 +107,9 @@ pub trait TemplateContext: Serialize { /// /// This is then used to check for template validity in unit tests and in /// the CLI (`cargo run -- templates check`) - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -144,9 +145,9 @@ pub(crate) fn sample_list(samples: Vec) -> BTreeMap( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -181,18 +182,20 @@ impl std::ops::Deref for WithLanguage { } impl TemplateContext for WithLanguage { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where Self: Sized, { + // Create a forked RNG so we make samples deterministic between locales + let rng = ChaCha8Rng::from_rng(rng).unwrap(); locales .iter() .flat_map(|locale| { - T::sample(now, rng, locales) + T::sample(now, &mut rng.clone(), locales) .into_iter() .map(|(sample_id, sample)| { ( @@ -218,9 +221,9 @@ pub struct WithCsrf { } impl TemplateContext for WithCsrf { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -251,9 +254,9 @@ pub struct WithSession { } impl TemplateContext for WithSession { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -289,9 +292,9 @@ pub struct WithOptionalSession { } impl TemplateContext for WithOptionalSession { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -340,9 +343,9 @@ impl Serialize for EmptyContext { } impl TemplateContext for EmptyContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -368,9 +371,9 @@ impl IndexContext { } impl TemplateContext for IndexContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -414,9 +417,9 @@ impl AppContext { } impl TemplateContext for AppContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -447,9 +450,9 @@ impl ApiDocContext { } impl TemplateContext for ApiDocContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -539,9 +542,9 @@ pub struct LoginContext { } impl TemplateContext for LoginContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -647,9 +650,9 @@ pub struct RegisterContext { } impl TemplateContext for RegisterContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -690,9 +693,9 @@ pub struct PasswordRegisterContext { } impl TemplateContext for PasswordRegisterContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -732,9 +735,9 @@ pub struct ConsentContext { } impl TemplateContext for ConsentContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -790,9 +793,9 @@ pub struct PolicyViolationContext { } impl TemplateContext for PolicyViolationContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -865,9 +868,9 @@ pub struct CompatSsoContext { } impl TemplateContext for CompatSsoContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -927,9 +930,9 @@ impl EmailRecoveryContext { } impl TemplateContext for EmailRecoveryContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -992,9 +995,9 @@ impl EmailVerificationContext { } impl TemplateContext for EmailVerificationContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1067,9 +1070,9 @@ impl RegisterStepsVerifyEmailContext { } impl TemplateContext for RegisterStepsVerifyEmailContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1107,9 +1110,9 @@ impl RegisterStepsEmailInUseContext { } impl TemplateContext for RegisterStepsEmailInUseContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1162,9 +1165,9 @@ impl RegisterStepsDisplayNameContext { } impl TemplateContext for RegisterStepsDisplayNameContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1217,9 +1220,9 @@ impl RegisterStepsRegistrationTokenContext { } impl TemplateContext for RegisterStepsRegistrationTokenContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1268,9 +1271,9 @@ impl RecoveryStartContext { } impl TemplateContext for RecoveryStartContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1310,9 +1313,9 @@ impl RecoveryProgressContext { } impl TemplateContext for RecoveryProgressContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1356,9 +1359,9 @@ impl RecoveryExpiredContext { } impl TemplateContext for RecoveryExpiredContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1420,9 +1423,9 @@ impl RecoveryFinishContext { } impl TemplateContext for RecoveryFinishContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1469,9 +1472,9 @@ impl UpstreamExistingLinkContext { } impl TemplateContext for UpstreamExistingLinkContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1507,9 +1510,9 @@ impl UpstreamSuggestLink { } impl TemplateContext for UpstreamSuggestLink { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1636,9 +1639,9 @@ impl UpstreamRegister { } impl TemplateContext for UpstreamRegister { - fn sample( + fn sample( now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1722,9 +1725,9 @@ impl DeviceLinkContext { } impl TemplateContext for DeviceLinkContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1756,9 +1759,9 @@ impl DeviceConsentContext { } impl TemplateContext for DeviceConsentContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1801,9 +1804,9 @@ impl AccountInactiveContext { } impl TemplateContext for AccountInactiveContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1837,9 +1840,9 @@ impl DeviceNameContext { } impl TemplateContext for DeviceNameContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1863,9 +1866,9 @@ pub struct FormPostContext { } impl TemplateContext for FormPostContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -1945,9 +1948,9 @@ impl std::fmt::Display for ErrorContext { } impl TemplateContext for ErrorContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -2039,9 +2042,9 @@ impl NotFoundContext { } impl TemplateContext for NotFoundContext { - fn sample( + fn sample( _now: DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where diff --git a/crates/templates/src/context/captcha.rs b/crates/templates/src/context/captcha.rs index 3daafb745..f9d8723b4 100644 --- a/crates/templates/src/context/captcha.rs +++ b/crates/templates/src/context/captcha.rs @@ -11,6 +11,7 @@ use minijinja::{ Value, value::{Enumerator, Object}, }; +use rand::Rng; use serde::Serialize; use crate::{TemplateContext, context::SampleIdentifier}; @@ -58,9 +59,9 @@ impl WithCaptcha { } impl TemplateContext for WithCaptcha { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl rand::prelude::Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where diff --git a/crates/templates/src/functions.rs b/crates/templates/src/functions.rs index 631e4742e..9d764e032 100644 --- a/crates/templates/src/functions.rs +++ b/crates/templates/src/functions.rs @@ -10,7 +10,7 @@ //! Additional functions, tests and filters used in templates use std::{ - collections::HashMap, + collections::{BTreeMap, HashMap}, fmt::Formatter, str::FromStr, sync::{Arc, atomic::AtomicUsize}, @@ -30,7 +30,7 @@ use url::Url; pub fn register( env: &mut minijinja::Environment, url_builder: UrlBuilder, - vite_manifest: ViteManifest, + vite_manifest: Option, translator: Arc, ) { env.set_unknown_method_callback(minijinja_contrib::pycompat::unknown_method_callback); @@ -43,13 +43,17 @@ pub fn register( env.add_filter("parse_user_agent", filter_parse_user_agent); env.add_function("add_params_to_url", function_add_params_to_url); env.add_function("counter", || Ok(Value::from_object(Counter::default()))); - env.add_global( - "include_asset", - Value::from_object(IncludeAsset { - url_builder: url_builder.clone(), - vite_manifest, - }), - ); + if let Some(vite_manifest) = vite_manifest { + env.add_global( + "include_asset", + Value::from_object(IncludeAsset { + url_builder: url_builder.clone(), + vite_manifest, + }), + ); + } else { + env.add_global("include_asset", Value::from_object(FakeIncludeAsset {})); + } env.add_global( "translator", Value::from_object(TranslatorFunc { translator }), @@ -182,7 +186,8 @@ fn function_add_params_to_url( .unwrap_or_default(); // Merge the exising and the additional parameters together - let params: HashMap<&String, &Value> = params.iter().chain(existing.iter()).collect(); + // Use a BTreeMap for determinism (because it orders keys) + let params: BTreeMap<&String, &Value> = params.iter().chain(existing.iter()).collect(); // Transform them back to urlencoded let params = serde_urlencoded::to_string(params).map_err(|e| { @@ -433,10 +438,10 @@ impl Object for IncludeAsset { let path: &Utf8Path = path.into(); - let (main, imported) = self.vite_manifest.find_assets(path).map_err(|_e| { + let (main, imported) = self.vite_manifest.find_assets(path).map_err(|e| { Error::new( ErrorKind::InvalidOperation, - "Invalid assets manifest while calling function `include_asset`", + format!("Invalid assets manifest while calling function `include_asset` with path = {path:?}: {e}"), ) })?; @@ -455,6 +460,30 @@ impl Object for IncludeAsset { } } +struct FakeIncludeAsset {} + +impl std::fmt::Debug for FakeIncludeAsset { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("FakeIncludeAsset").finish() + } +} + +impl std::fmt::Display for FakeIncludeAsset { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("fake_include_asset") + } +} + +impl Object for FakeIncludeAsset { + fn call(self: &Arc, _state: &State, args: &[Value]) -> Result { + let (path,): (&str,) = from_args(args)?; + + Ok(Value::from_safe_string(format!( + "" + ))) + } +} + #[derive(Debug, Default)] struct Counter { count: AtomicUsize, diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index 603dcfdf6..32a41e8b2 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -72,7 +72,7 @@ pub struct Templates { url_builder: UrlBuilder, branding: SiteBranding, features: SiteFeatures, - vite_manifest_path: Utf8PathBuf, + vite_manifest_path: Option, translations_path: Utf8PathBuf, path: Utf8PathBuf, /// Whether template rendering is in strict mode (for testing, @@ -143,6 +143,11 @@ fn is_hidden(entry: &DirEntry) -> bool { impl Templates { /// Load the templates from the given config /// + /// # Parameters + /// + /// - `vite_manifest_path`: None if we are rendering resources for + /// reproducibility, in which case a dummy Vite manifest will be used. + /// /// # Errors /// /// Returns an error if the templates could not be loaded from disk. @@ -154,7 +159,7 @@ impl Templates { pub async fn load( path: Utf8PathBuf, url_builder: UrlBuilder, - vite_manifest_path: Utf8PathBuf, + vite_manifest_path: Option, translations_path: Utf8PathBuf, branding: SiteBranding, features: SiteFeatures, @@ -163,7 +168,7 @@ impl Templates { let (translator, environment) = Self::load_( &path, url_builder.clone(), - &vite_manifest_path, + vite_manifest_path.as_deref(), &translations_path, branding.clone(), features, @@ -186,7 +191,7 @@ impl Templates { async fn load_( path: &Utf8Path, url_builder: UrlBuilder, - vite_manifest_path: &Utf8Path, + vite_manifest_path: Option<&Utf8Path>, translations_path: &Utf8Path, branding: SiteBranding, features: SiteFeatures, @@ -196,13 +201,20 @@ impl Templates { let span = tracing::Span::current(); // Read the assets manifest from disk - let vite_manifest = tokio::fs::read(vite_manifest_path) - .await - .map_err(TemplateLoadingError::ViteManifestIO)?; + let vite_manifest = if let Some(vite_manifest_path) = vite_manifest_path { + let raw_vite_manifest = tokio::fs::read(vite_manifest_path) + .await + .map_err(TemplateLoadingError::ViteManifestIO)?; + + Some( + serde_json::from_slice::(&raw_vite_manifest) + .map_err(TemplateLoadingError::ViteManifest)?, + ) + } else { + None + }; // Parse it - let vite_manifest: ViteManifest = - serde_json::from_slice(&vite_manifest).map_err(TemplateLoadingError::ViteManifest)?; let translations_path = translations_path.to_owned(); let translator = @@ -291,7 +303,7 @@ impl Templates { let (translator, environment) = Self::load_( &self.path, self.url_builder.clone(), - &self.vite_manifest_path, + self.vite_manifest_path.as_deref(), &self.translations_path, self.branding.clone(), self.features, @@ -471,10 +483,10 @@ impl Templates { /// # Errors /// /// Returns an error if any of the templates fails to render - pub fn check_render( + pub fn check_render( &self, now: chrono::DateTime, - rng: &mut impl Rng, + rng: &R, ) -> anyhow::Result> { check::all(self, now, rng) } @@ -482,14 +494,15 @@ impl Templates { #[cfg(test)] mod tests { + use rand::SeedableRng; + use super::*; #[tokio::test] async fn check_builtin_templates() { #[allow(clippy::disallowed_methods)] let now = chrono::Utc::now(); - #[allow(clippy::disallowed_methods)] - let mut rng = rand::thread_rng(); + let rng = rand_chacha::ChaCha8Rng::from_seed([42; 32]); let path = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../templates/"); let url_builder = UrlBuilder::new("https://example.com/".parse().unwrap(), None, None); @@ -505,18 +518,28 @@ mod tests { Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../frontend/dist/manifest.json"); let translations_path = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../translations"); - let templates = Templates::load( - path, - url_builder, - vite_manifest_path, - translations_path, - branding, - features, - // Use strict mode in tests - true, - ) - .await - .unwrap(); - templates.check_render(now, &mut rng).unwrap(); + + for use_real_vite_manifest in [true, false] { + let templates = Templates::load( + path.clone(), + url_builder.clone(), + // Check both renders against the real vite manifest and the 'dummy' vite manifest + // used for reproducible renders. + use_real_vite_manifest.then_some(vite_manifest_path.clone()), + translations_path.clone(), + branding.clone(), + features, + // Use strict mode in tests + true, + ) + .await + .unwrap(); + + // Check the renders are deterministic, when given the same rng + let render1 = templates.check_render(now, &rng).unwrap(); + let render2 = templates.check_render(now, &rng).unwrap(); + + assert_eq!(render1, render2); + } } } diff --git a/crates/templates/src/macros.rs b/crates/templates/src/macros.rs index 95b57f0d9..c6d5e9302 100644 --- a/crates/templates/src/macros.rs +++ b/crates/templates/src/macros.rs @@ -78,15 +78,18 @@ macro_rules! register_templates { /// # Errors /// /// Returns an error if any template fails to render with any of the sample. - pub(crate) fn all(templates: &Templates, now: chrono::DateTime, rng: &mut impl rand::Rng) -> anyhow::Result<::std::collections::BTreeMap<(&'static str, SampleIdentifier), String>> { + pub(crate) fn all(templates: &Templates, now: chrono::DateTime, rng: &R) -> anyhow::Result<::std::collections::BTreeMap<(&'static str, SampleIdentifier), String>> { let mut out = ::std::collections::BTreeMap::new(); // TODO shouldn't the Rng be independent for each render? $( - out.extend( - $name $(::< $( $generic_default ),* >)? (templates, now, rng)? - .into_iter() - .map(|(sample_identifier, rendered)| (($template, sample_identifier), rendered)) - ); + { + let mut rng = rng.clone(); + out.extend( + $name $(::< _ $( , $generic_default ),* >)? (templates, now, &mut rng)? + .into_iter() + .map(|(sample_identifier, rendered)| (($template, sample_identifier), rendered)) + ); + } )* Ok(out) @@ -101,8 +104,8 @@ macro_rules! register_templates { /// /// Returns an error if the template fails to render with any of the sample. pub(crate) fn $name - $(< $( $lt $( : $clt $(+ $dlt )* + TemplateContext )? ),+ >)? - (templates: &Templates, now: chrono::DateTime, rng: &mut impl rand::Rng) + < __R: Rng + Clone $( , $( $lt $( : $clt $(+ $dlt )* + TemplateContext )? ),+ )? > + (templates: &Templates, now: chrono::DateTime, rng: &mut __R) -> anyhow::Result> { let locales = templates.translator().available_locales(); let samples: BTreeMap = TemplateContext::sample(now, rng, &locales); diff --git a/docs/reference/cli/templates.md b/docs/reference/cli/templates.md index ccced6bed..9877f3af9 100644 --- a/docs/reference/cli/templates.md +++ b/docs/reference/cli/templates.md @@ -17,3 +17,17 @@ INFO mas_core::templates::check: Rendering template name="register.html" context INFO mas_core::templates::check: Rendering template name="index.html" context={"csrf_token":"fake_csrf_token","current_session":{"active":true,"created_at":"2021-09-24T13:26:52.962135085Z","id":1,"last_authd_at":"2021-09-24T13:26:52.962135316Z","user_id":2,"username":"john"},"discovery_url":"https://example.com/.well-known/openid-configuration"} ... ``` + +Options: +- `--out-dir `: Render templates and emit them to the specified directory, which must either not exist or be empty. +- `--stabilise`: Remove sources of nondeterminism from template inputs, so that renders are reproducible. Only useful with `--out-dir`. + +What is checked: +- the Jinja templates are syntactically valid +- the templates can render with a few sample values, with the branding from the MAS configuration + - undefined variables (`{{ undefined_variable }}`) will raise errors +- all translation keys exist + +What is not checked: +- the validity of the generated HTML (you can forget closing tags, or otherwise create invalid HTML output) +- all translation keys exist *in your intended language(s)* (so some translation keys may fall back to English)