From 267cb9396feb4428e1c567811522a7946e8adc09 Mon Sep 17 00:00:00 2001 From: Sergio Benitez Date: Fri, 21 May 2021 22:51:14 -0700 Subject: [PATCH] Introduce 'Singleton' fairings. A singleton fairing is guaranteed to be the only instance of its type at launch time. If more than one instance of a singleton fairing is attached, only the last instance is retained. --- core/lib/src/fairing/fairings.rs | 107 ++++++++++++++---- core/lib/src/fairing/info_kind.rs | 13 ++- core/lib/src/fairing/mod.rs | 28 +++-- core/lib/tests/recursive-singleton-fairing.rs | 82 ++++++++++++++ site/guide/7-fairings.md | 13 ++- 5 files changed, 204 insertions(+), 39 deletions(-) create mode 100644 core/lib/tests/recursive-singleton-fairing.rs diff --git a/core/lib/src/fairing/fairings.rs b/core/lib/src/fairing/fairings.rs index 2796c1ba..afbbfc57 100644 --- a/core/lib/src/fairing/fairings.rs +++ b/core/lib/src/fairing/fairings.rs @@ -1,3 +1,5 @@ +use std::collections::HashSet; + use crate::{Rocket, Request, Response, Data, Build, Orbit}; use crate::fairing::{Fairing, Info, Kind}; use crate::log::PaintExt; @@ -6,12 +8,14 @@ use yansi::Paint; #[derive(Default)] pub struct Fairings { + // NOTE: This is a push-only vector due to the index-vectors below! all_fairings: Vec>, + // Ignite fairings that have failed. failures: Vec, - // Index into `attach` of last run attach fairing. - last_launch: usize, + // The number of ignite fairings from `self.ignite` we've run. + num_ignited: usize, // The vectors below hold indices into `all_fairings`. - launch: Vec, + ignite: Vec, liftoff: Vec, request: Vec, response: Vec, @@ -19,8 +23,15 @@ pub struct Fairings { macro_rules! iter { ($_self:ident . $kind:ident) => ({ + iter!($_self, $_self.$kind.iter()).map(|v| v.1) + }); + ($_self:ident, $indices:expr) => ({ let all_fairings = &$_self.all_fairings; - $_self.$kind.iter().filter_map(move |i| all_fairings.get(*i).map(|f| &**f)) + $indices.filter_map(move |i| { + debug_assert!(all_fairings.get(*i).is_some()); + let f = all_fairings.get(*i).map(|f| &**f)?; + Some((*i, f)) + }) }) } @@ -30,17 +41,64 @@ impl Fairings { Fairings::default() } - pub fn add(&mut self, fairing: Box) -> &dyn Fairing { - let kind = fairing.info().kind; + pub fn active(&self) -> impl Iterator { + self.ignite.iter() + .chain(self.liftoff.iter()) + .chain(self.request.iter()) + .chain(self.response.iter()) + } + + pub fn add(&mut self, fairing: Box) { + let this = &fairing; + let this_info = this.info(); + if this_info.kind.is(Kind::Singleton) { + // If we already ran a duplicate on ignite, then fail immediately. + // There is no way to uphold the "only run last singleton" promise. + // + // How can this happen? Like this: + // 1. Attach A (singleton). + // 2. Attach B (any fairing). + // 3. Ignite. + // 4. A executes on_ignite. + // 5. B executes on_ignite, attaches another A. + // 6. --- (A would run if not for this code) + let ignite_dup = iter!(self.ignite).position(|f| f.type_id() == this.type_id()); + if let Some(dup_ignite_index) = ignite_dup { + if dup_ignite_index < self.num_ignited { + self.failures.push(this_info); + return; + } + } + + // Finds `k` in `from` and removes it if it's there. + let remove = |k: usize, from: &mut Vec| { + if let Ok(j) = from.binary_search(&k) { + from.remove(j); + } + }; + + // Collect all of the active duplicates. + let mut dups: Vec = iter!(self, self.active()) + .filter(|(_, f)| f.type_id() == this.type_id()) + .map(|(i, _)| i) + .collect(); + + // Reverse the dup indices so `remove` is stable given shifts. + dups.sort(); dups.dedup(); dups.reverse(); + for i in dups { + remove(i, &mut self.ignite); + remove(i, &mut self.liftoff); + remove(i, &mut self.request); + remove(i, &mut self.response); + } + } + let index = self.all_fairings.len(); self.all_fairings.push(fairing); - - if kind.is(Kind::Ignite) { self.launch.push(index); } - if kind.is(Kind::Liftoff) { self.liftoff.push(index); } - if kind.is(Kind::Request) { self.request.push(index); } - if kind.is(Kind::Response) { self.response.push(index); } - - &*self.all_fairings[index] + if this_info.kind.is(Kind::Ignite) { self.ignite.push(index); } + if this_info.kind.is(Kind::Liftoff) { self.liftoff.push(index); } + if this_info.kind.is(Kind::Request) { self.request.push(index); } + if this_info.kind.is(Kind::Response) { self.response.push(index); } } pub fn append(&mut self, others: &mut Fairings) { @@ -50,10 +108,10 @@ impl Fairings { } pub async fn handle_ignite(mut rocket: Rocket) -> Rocket { - while rocket.fairings.last_launch < rocket.fairings.launch.len() { + while rocket.fairings.num_ignited < rocket.fairings.ignite.len() { // We're going to move `rocket` while borrowing `fairings`... let mut fairings = std::mem::replace(&mut rocket.fairings, Fairings::new()); - for fairing in iter!(fairings.launch).skip(fairings.last_launch) { + for fairing in iter!(fairings.ignite).skip(fairings.num_ignited) { let info = fairing.info(); rocket = match fairing.on_ignite(rocket).await { Ok(rocket) => rocket, @@ -63,10 +121,10 @@ impl Fairings { } }; - fairings.last_launch += 1; + fairings.num_ignited += 1; } - // Note that `rocket.fairings` may now be non-empty since launch + // Note that `rocket.fairings` may now be non-empty since ignite // fairings could have added more fairings! Move them to the end. fairings.append(&mut rocket.fairings); rocket.fairings = fairings; @@ -89,9 +147,9 @@ impl Fairings { } #[inline(always)] - pub async fn handle_response<'r>(&self, request: &'r Request<'_>, response: &mut Response<'r>) { + pub async fn handle_response<'r>(&self, req: &'r Request<'_>, res: &mut Response<'r>) { for fairing in iter!(self.response) { - fairing.on_response(request, response).await; + fairing.on_response(req, res).await; } } @@ -103,13 +161,14 @@ impl Fairings { } pub fn pretty_print(&self) { - if !self.all_fairings.is_empty() { + let active_fairings = self.active().collect::>(); + if !active_fairings.is_empty() { launch_info!("{}{}:", Paint::emoji("📡 "), Paint::magenta("Fairings")); - } - for fairing in &self.all_fairings { - launch_info_!("{} ({})", Paint::default(fairing.info().name).bold(), + for (_, fairing) in iter!(self, active_fairings.into_iter()) { + launch_info_!("{} ({})", Paint::default(fairing.info().name).bold(), Paint::blue(fairing.info().kind).bold()); + } } } } @@ -121,7 +180,7 @@ impl std::fmt::Debug for Fairings { } f.debug_struct("Fairings") - .field("launch", &debug_info(iter!(self.launch))) + .field("launch", &debug_info(iter!(self.ignite))) .field("liftoff", &debug_info(iter!(self.liftoff))) .field("request", &debug_info(iter!(self.request))) .field("response", &debug_info(iter!(self.response))) diff --git a/core/lib/src/fairing/info_kind.rs b/core/lib/src/fairing/info_kind.rs index 67affc02..adaa129e 100644 --- a/core/lib/src/fairing/info_kind.rs +++ b/core/lib/src/fairing/info_kind.rs @@ -27,7 +27,7 @@ pub struct Info { /// The name of the fairing. pub name: &'static str, /// A set representing the callbacks the fairing wishes to receive. - pub kind: Kind + pub kind: Kind, } /// A bitset representing the kinds of callbacks a @@ -45,6 +45,10 @@ pub struct Info { /// instance, to represent a fairing that is both an ignite and request fairing, /// use `Kind::Ignite | Kind::Request`. Similarly, to represent a fairing that /// is only an ignite fairing, use `Kind::Ignite`. +/// +/// Additionally, a fairing can request to be treated as a +/// [singleton](crate::fairing::Fairing#singletons) by specifying the +/// `Singleton` kind. #[derive(Debug, Clone, Copy)] pub struct Kind(usize); @@ -62,6 +66,10 @@ impl Kind { /// `Kind` flag representing a request for a 'response' callback. pub const Response: Kind = Kind(1 << 3); + /// `Kind` flag representing a + /// [singleton](crate::fairing::Fairing#singletons) fairing. + pub const Singleton: Kind = Kind(1 << 4); + /// Returns `true` if `self` is a superset of `other`. In other words, /// returns `true` if all of the kinds in `other` are also in `self`. /// @@ -132,6 +140,7 @@ impl std::fmt::Display for Kind { write("ignite", Kind::Ignite)?; write("liftoff", Kind::Liftoff)?; write("request", Kind::Request)?; - write("response", Kind::Response) + write("response", Kind::Response)?; + write("singleton", Kind::Singleton) } } diff --git a/core/lib/src/fairing/mod.rs b/core/lib/src/fairing/mod.rs index 18e427cf..e73d5ce2 100644 --- a/core/lib/src/fairing/mod.rs +++ b/core/lib/src/fairing/mod.rs @@ -38,15 +38,19 @@ //! ## Ordering //! //! `Fairing`s are executed in the order in which they are attached: the first -//! attached fairing has its callbacks executed before all others. Because -//! fairing callbacks may not be commutative, the order in which fairings are -//! attached may be significant. Because of this, it is important to communicate -//! to the user every consequence of a fairing. +//! attached fairing has its callbacks executed before all others. A fairing can +//! be attached any number of times. Except for [singleton +//! fairings](Fairing#singletons), all attached instances are polled at runtime. +//! Fairing callbacks may not be commutative; the order in which fairings are +//! attached may be significant. It is thus important to communicate specific +//! fairing functionality clearly. //! //! Furthermore, a `Fairing` should take care to act locally so that the actions //! of other `Fairings` are not jeopardized. For instance, unless it is made //! abundantly clear, a fairing should not rewrite every request. +use std::any::Any; + use crate::{Rocket, Request, Response, Data, Build, Orbit}; mod fairings; @@ -99,8 +103,8 @@ pub type Result, E = Rocket> = std::result::Result, E = Rocket> = std::result::Result, E = Rocket> = std::result::Result Info { + Info { + name: "Singleton", + kind: self.0 + } + } + + async fn on_ignite(&self, rocket: Rocket) -> fairing::Result { + if self.2 { + Ok(rocket.attach(Singleton(self.1, self.1, false))) + } else { + Ok(rocket) + } + } +} + + +// Have => two `Singleton`s. This is okay; we keep the latter. +#[rocket::async_test] +async fn recursive_singleton_ok() { + let result = rocket::custom(Config::debug_default()) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Singleton, false)) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Singleton, false)) + .ignite() + .await; + + assert!(result.is_ok(), "{:?}", result); + + let result = rocket::custom(Config::debug_default()) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Singleton, false)) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Singleton, false)) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Singleton, false)) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Singleton, false)) + .ignite() + .await; + + assert!(result.is_ok(), "{:?}", result); +} + +// Have a `Singleton` add itself `on_ignite()`. Since it already ran, the one it +// adds can't be unique, so ensure we error in this case. +#[rocket::async_test] +async fn recursive_singleton_bad() { + #[track_caller] + fn assert_err(error: rocket::Error) { + if let ErrorKind::FailedFairings(v) = error.kind() { + assert_eq!(v.len(), 1); + assert_eq!(v[0].name, "Singleton"); + } else { + panic!("unexpected error: {:?}", error); + } + } + + let result = rocket::custom(Config::debug_default()) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Ignite | Kind::Singleton, true)) + .ignite() + .await; + + assert_err(result.unwrap_err()); + + let result = rocket::custom(Config::debug_default()) + .attach(Singleton(Kind::Ignite | Kind::Singleton, Kind::Singleton, true)) + .ignite() + .await; + + assert_err(result.unwrap_err()); + + let result = rocket::custom(Config::debug_default()) + .attach(Singleton(Kind::Ignite, Kind::Singleton, true)) + .ignite() + .await; + + assert_err(result.unwrap_err()); +} diff --git a/site/guide/7-fairings.md b/site/guide/7-fairings.md index 4704bdf9..55e6dbd5 100644 --- a/site/guide/7-fairings.md +++ b/site/guide/7-fairings.md @@ -54,21 +54,22 @@ example, the following snippet attached two fairings, `req_fairing` and fn rocket() -> _ { # let req_fairing = rocket::fairing::AdHoc::on_request("example", |_, _| Box::pin(async {})); # let res_fairing = rocket::fairing::AdHoc::on_response("example", |_, _| Box::pin(async {})); - rocket::build() .attach(req_fairing) .attach(res_fairing) } ``` +Fairings are executed in the order in which they are attached: the first +attached fairing has its callbacks executed before all others. A fairing can be +attached any number of times. Except for [singleton fairings], all attached +instances are polled at runtime. Fairing callbacks may not be commutative; the +order in which fairings are attached may be significant. + +[singleton fairings]: @api/rocket/fairing/trait.Fairing.html#singletons [`attach`]: @api/rocket/struct.Rocket.html#method.attach [`Rocket`]: @api/rocket/struct.Rocket.html -Fairings are executed in the order in which they are attached: the first -attached fairing has its callbacks executed before all others. Because fairing -callbacks may not be commutative, the order in which fairings are attached may -be significant. - ### Callbacks There are four events for which Rocket issues fairing callbacks. Each of these