diff --git a/crates/cli/src/commands/templates.rs b/crates/cli/src/commands/templates.rs index 8f1b0dd4e..75acce21d 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(0).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( @@ -79,7 +94,7 @@ impl Options { true, ) .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/templates/src/context.rs b/crates/templates/src/context.rs index 597683a03..c32691c4c 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -106,9 +106,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 +144,9 @@ pub(crate) fn sample_list(samples: Vec) -> BTreeMap( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -181,9 +181,9 @@ 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 @@ -192,7 +192,9 @@ impl TemplateContext for WithLanguage { locales .iter() .flat_map(|locale| { - T::sample(now, rng, locales) + // Make samples deterministic between locales + let mut rng = rng.clone(); + T::sample(now, &mut rng, locales) .into_iter() .map(|(sample_id, sample)| { ( @@ -218,9 +220,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 +253,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 +291,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 +342,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 +370,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 +416,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 +449,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 +541,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 +649,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 +692,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 +734,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 +792,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 +867,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 +929,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 +994,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 +1069,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 +1109,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 +1164,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 +1219,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 +1270,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 +1312,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 +1358,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 +1422,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 +1471,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 +1509,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 +1638,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 +1724,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 +1758,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 +1803,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 +1839,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 +1865,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 +1947,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 +2041,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..32c2d1068 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/lib.rs b/crates/templates/src/lib.rs index 603dcfdf6..a57f718f0 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -471,10 +471,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) } @@ -489,7 +489,7 @@ mod tests { #[allow(clippy::disallowed_methods)] let now = chrono::Utc::now(); #[allow(clippy::disallowed_methods)] - let mut rng = rand::thread_rng(); + let rng = rand::thread_rng(); let path = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../templates/"); let url_builder = UrlBuilder::new("https://example.com/".parse().unwrap(), None, None); @@ -517,6 +517,6 @@ mod tests { ) .await .unwrap(); - templates.check_render(now, &mut rng).unwrap(); + templates.check_render(now, &rng).unwrap(); } } 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);