diff --git a/src/bin/cratesfyi.rs b/src/bin/cratesfyi.rs index cc374611f..d30a29723 100644 --- a/src/bin/cratesfyi.rs +++ b/src/bin/cratesfyi.rs @@ -2,11 +2,11 @@ use std::env; use std::path::PathBuf; use std::sync::Arc; -use cratesfyi::db::{self, add_path_into_database, connect_db}; +use cratesfyi::db::{self, add_path_into_database, Pool}; use cratesfyi::utils::{add_crate_to_queue, remove_crate_priority, set_crate_priority}; -use cratesfyi::Server; -use cratesfyi::{DocBuilder, DocBuilderOptions, RustwideBuilder}; +use cratesfyi::{Config, DocBuilder, DocBuilderOptions, RustwideBuilder, Server}; use failure::Error; +use postgres::Connection; use structopt::StructOpt; pub fn main() -> Result<(), Error> { @@ -82,25 +82,26 @@ enum CommandLine { impl CommandLine { pub fn handle_args(self) -> Result<(), Error> { - let config = Arc::new(cratesfyi::Config::from_env()?); + let config = Arc::new(Config::from_env()?); + let pool = Pool::new(&config)?; match self { - Self::Build(build) => build.handle_args(), + Self::Build(build) => build.handle_args(pool), Self::StartWebServer { socket_addr, reload_templates, } => { - Server::start(Some(&socket_addr), reload_templates, config)?; + Server::start(Some(&socket_addr), reload_templates, pool, config)?; } Self::Daemon { foreground } => { if foreground { log::warn!("--foreground was passed, but there is no need for it anymore"); } - cratesfyi::utils::start_daemon(config)?; + cratesfyi::utils::start_daemon(config, pool)?; } - Self::Database { subcommand } => subcommand.handle_args(), - Self::Queue { subcommand } => subcommand.handle_args(), + Self::Database { subcommand } => subcommand.handle_args(&*pool.get()?), + Self::Queue { subcommand } => subcommand.handle_args(&*pool.get()?), } Ok(()) @@ -135,19 +136,18 @@ enum QueueSubcommand { } impl QueueSubcommand { - pub fn handle_args(self) { + pub fn handle_args(self, conn: &Connection) { match self { Self::Add { crate_name, crate_version, build_priority, } => { - let conn = connect_db().expect("Could not connect to database"); add_crate_to_queue(&conn, &crate_name, &crate_version, build_priority) .expect("Could not add crate to queue"); } - Self::DefaultPriority { subcommand } => subcommand.handle_args(), + Self::DefaultPriority { subcommand } => subcommand.handle_args(conn), } } } @@ -172,18 +172,14 @@ enum PrioritySubcommand { } impl PrioritySubcommand { - pub fn handle_args(self) { + pub fn handle_args(self, conn: &Connection) { match self { Self::Set { pattern, priority } => { - let conn = connect_db().expect("Could not connect to the database"); - set_crate_priority(&conn, &pattern, priority) .expect("Could not set pattern's priority"); } Self::Remove { pattern } => { - let conn = connect_db().expect("Could not connect to the database"); - if let Some(priority) = remove_crate_priority(&conn, &pattern) .expect("Could not remove pattern's priority") { @@ -237,7 +233,7 @@ struct Build { } impl Build { - pub fn handle_args(self) { + pub fn handle_args(self, pool: Pool) { let docbuilder = { let mut doc_options = DocBuilderOptions::from_prefix(self.prefix); @@ -253,10 +249,10 @@ impl Build { .check_paths() .expect("The given paths were invalid"); - DocBuilder::new(doc_options) + DocBuilder::new(doc_options, pool.clone()) }; - self.subcommand.handle_args(docbuilder); + self.subcommand.handle_args(docbuilder, pool); } } @@ -304,12 +300,12 @@ enum BuildSubcommand { } impl BuildSubcommand { - pub fn handle_args(self, mut docbuilder: DocBuilder) { + pub fn handle_args(self, mut docbuilder: DocBuilder, pool: cratesfyi::db::Pool) { match self { Self::World => { docbuilder.load_cache().expect("Failed to load cache"); - let mut builder = RustwideBuilder::init().unwrap(); + let mut builder = RustwideBuilder::init(pool).unwrap(); builder .build_world(&mut docbuilder) .expect("Failed to build world"); @@ -323,7 +319,8 @@ impl BuildSubcommand { local, } => { docbuilder.load_cache().expect("Failed to load cache"); - let mut builder = RustwideBuilder::init().expect("failed to initialize rustwide"); + let mut builder = + RustwideBuilder::init(pool).expect("failed to initialize rustwide"); if let Some(path) = local { builder @@ -345,7 +342,7 @@ impl BuildSubcommand { Self::UpdateToolchain { only_first_time } => { if only_first_time { - let conn = db::connect_db().unwrap(); + let conn = pool.get().expect("failed to get a database connection"); let res = conn .query("SELECT * FROM config WHERE name = 'rustc_version';", &[]) .unwrap(); @@ -356,14 +353,14 @@ impl BuildSubcommand { } } - let mut builder = RustwideBuilder::init().unwrap(); + let mut builder = RustwideBuilder::init(pool).unwrap(); builder .update_toolchain() .expect("failed to update toolchain"); } Self::AddEssentialFiles => { - let mut builder = RustwideBuilder::init().unwrap(); + let mut builder = RustwideBuilder::init(pool).unwrap(); builder .add_essential_files() .expect("failed to add essential files"); @@ -378,7 +375,7 @@ impl BuildSubcommand { #[derive(Debug, Clone, PartialEq, Eq, StructOpt)] enum DatabaseSubcommand { - /// Run database migrations + /// Run database migration Migrate { /// The database version to migrate to #[structopt(name = "VERSION")] @@ -415,33 +412,30 @@ enum DatabaseSubcommand { } impl DatabaseSubcommand { - pub fn handle_args(self) { + pub fn handle_args(self, conn: &Connection) { match self { Self::Migrate { version } => { - let conn = connect_db().expect("failed to connect to the database"); db::migrate(version, &conn).expect("Failed to run database migrations"); } Self::UpdateGithubFields => { - cratesfyi::utils::github_updater().expect("Failed to update github fields"); + cratesfyi::utils::github_updater(&conn).expect("Failed to update github fields"); } Self::AddDirectory { directory, prefix } => { - let conn = db::connect_db().expect("failed to connect to the database"); add_path_into_database(&conn, &prefix, directory) .expect("Failed to add directory into database"); } // FIXME: This is actually util command not database - Self::UpdateReleaseActivity => cratesfyi::utils::update_release_activity() + Self::UpdateReleaseActivity => cratesfyi::utils::update_release_activity(&conn) .expect("Failed to update release activity"), Self::DeleteCrate { crate_name } => { - let conn = db::connect_db().expect("failed to connect to the database"); db::delete_crate(&conn, &crate_name).expect("failed to delete the crate"); } - Self::Blacklist { command } => command.handle_args(), + Self::Blacklist { command } => command.handle_args(&conn), } } } @@ -467,9 +461,7 @@ enum BlacklistSubcommand { } impl BlacklistSubcommand { - fn handle_args(self) { - let conn = db::connect_db().expect("failed to connect to the database"); - + fn handle_args(self, conn: &Connection) { match self { Self::List => { let crates = diff --git a/src/config.rs b/src/config.rs index ff3a87d67..85920639d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,10 +1,15 @@ -use failure::{bail, Error, Fail, ResultExt}; +use failure::{bail, format_err, Error, Fail, ResultExt}; use std::env::VarError; use std::str::FromStr; -use std::sync::Arc; #[derive(Debug)] pub struct Config { + // Database connection params + pub(crate) database_url: String, + pub(crate) max_pool_size: u32, + pub(crate) min_pool_idle: u32, + + // Max size of the files served by the docs.rs frontend pub(crate) max_file_size: usize, pub(crate) max_file_size_html: usize, } @@ -12,17 +17,33 @@ pub struct Config { impl Config { pub fn from_env() -> Result { Ok(Self { + database_url: require_env("CRATESFYI_DATABASE_URL")?, + max_pool_size: env("DOCSRS_MAX_POOL_SIZE", 90)?, + min_pool_idle: env("DOCSRS_MIN_POOL_IDLE", 10)?, + max_file_size: env("DOCSRS_MAX_FILE_SIZE", 50 * 1024 * 1024)?, max_file_size_html: env("DOCSRS_MAX_FILE_SIZE_HTML", 5 * 1024 * 1024)?, }) } } -impl iron::typemap::Key for Config { - type Value = Arc; +fn env(var: &str, default: T) -> Result +where + T: FromStr, + T::Err: Fail, +{ + Ok(maybe_env(var)?.unwrap_or(default)) +} + +fn require_env(var: &str) -> Result +where + T: FromStr, + T::Err: Fail, +{ + maybe_env(var)?.ok_or_else(|| format_err!("configuration variable {} is missing", var)) } -fn env(var: &str, default: T) -> Result +fn maybe_env(var: &str) -> Result, Error> where T: FromStr, T::Err: Fail, @@ -30,8 +51,9 @@ where match std::env::var(var) { Ok(content) => Ok(content .parse::() + .map(Some) .with_context(|_| format!("failed to parse configuration variable {}", var))?), - Err(VarError::NotPresent) => Ok(default), + Err(VarError::NotPresent) => Ok(None), Err(VarError::NotUnicode(_)) => bail!("configuration variable {} is not UTF-8", var), } } diff --git a/src/db/mod.rs b/src/db/mod.rs index cf5cb52ec..e3679598b 100644 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -5,62 +5,11 @@ pub(crate) use self::add_package::add_package_into_database; pub use self::delete_crate::delete_crate; pub use self::file::add_path_into_database; pub use self::migrate::migrate; - -use failure::Fail; -use postgres::{Connection, TlsMode}; -use std::env; +pub use self::pool::{Pool, PoolError}; mod add_package; pub mod blacklist; mod delete_crate; pub(crate) mod file; mod migrate; - -/// Connects to database -pub fn connect_db() -> Result { - let err = "CRATESFYI_DATABASE_URL environment variable is not set"; - let db_url = env::var("CRATESFYI_DATABASE_URL").map_err(|e| e.context(err))?; - Connection::connect(&db_url[..], TlsMode::None).map_err(Into::into) -} - -pub(crate) fn create_pool() -> r2d2::Pool { - let db_url = env::var("CRATESFYI_DATABASE_URL") - .expect("CRATESFYI_DATABASE_URL environment variable is not exists"); - - let max_pool_size = env::var("DOCSRS_MAX_POOL_SIZE") - .map(|s| { - s.parse::() - .expect("DOCSRS_MAX_POOL_SIZE must be an integer") - }) - .unwrap_or(90); - crate::web::metrics::MAX_DB_CONNECTIONS.set(max_pool_size as i64); - - let min_pool_idle = env::var("DOCSRS_MIN_POOL_IDLE") - .map(|s| { - s.parse::() - .expect("DOCSRS_MIN_POOL_IDLE must be an integer") - }) - .unwrap_or(10); - - let manager = - r2d2_postgres::PostgresConnectionManager::new(&db_url[..], r2d2_postgres::TlsMode::None) - .expect("Failed to create PostgresConnectionManager"); - - r2d2::Pool::builder() - .max_size(max_pool_size) - .min_idle(Some(min_pool_idle)) - .build(manager) - .expect("Failed to create r2d2 pool") -} - -#[cfg(test)] -mod test { - use super::*; - - #[test] - #[ignore] - fn test_connect_db() { - let conn = connect_db(); - assert!(conn.is_ok()); - } -} +mod pool; diff --git a/src/web/pool.rs b/src/db/pool.rs similarity index 51% rename from src/web/pool.rs rename to src/db/pool.rs index 1c827f6bb..79710ae95 100644 --- a/src/web/pool.rs +++ b/src/db/pool.rs @@ -1,5 +1,4 @@ -use crate::db::create_pool; -use iron::{status::Status, typemap, IronError, IronResult}; +use crate::Config; use postgres::Connection; use std::marker::PhantomData; @@ -7,15 +6,29 @@ use std::marker::PhantomData; use std::sync::{Arc, Mutex, MutexGuard}; #[derive(Debug, Clone)] -pub(crate) enum Pool { +pub enum Pool { R2D2(r2d2::Pool), #[cfg(test)] Simple(Arc>), } impl Pool { - pub(crate) fn new() -> Pool { - Pool::R2D2(create_pool()) + pub fn new(config: &Config) -> Result { + crate::web::metrics::MAX_DB_CONNECTIONS.set(config.max_pool_size as i64); + + let manager = r2d2_postgres::PostgresConnectionManager::new( + config.database_url.as_str(), + r2d2_postgres::TlsMode::None, + ) + .map_err(PoolError::InvalidDatabaseUrl)?; + + let pool = r2d2::Pool::builder() + .max_size(config.max_pool_size) + .min_idle(Some(config.min_pool_idle)) + .build(manager) + .map_err(PoolError::PoolCreationFailed)?; + + Ok(Pool::R2D2(pool)) } #[cfg(test)] @@ -23,18 +36,15 @@ impl Pool { Pool::Simple(conn) } - pub(super) fn get<'a>(&'a self) -> IronResult> { + pub fn get(&self) -> Result, PoolError> { match self { - Self::R2D2(conn) => { - let conn = conn.get().map_err(|err| { - log::error!("Error getting db connection: {:?}", err); - super::metrics::FAILED_DB_CONNECTIONS.inc(); - - IronError::new(err, Status::InternalServerError) - })?; - - Ok(DerefConnection::Connection(conn, PhantomData)) - } + Self::R2D2(r2d2) => match r2d2.get() { + Ok(conn) => Ok(DerefConnection::Connection(conn, PhantomData)), + Err(err) => { + crate::web::metrics::FAILED_DB_CONNECTIONS.inc(); + Err(PoolError::ConnectionError(err)) + } + }, #[cfg(test)] Self::Simple(mutex) => Ok(DerefConnection::Guard( @@ -62,11 +72,7 @@ impl Pool { } } -impl typemap::Key for Pool { - type Value = Pool; -} - -pub(crate) enum DerefConnection<'a> { +pub enum DerefConnection<'a> { Connection( r2d2::PooledConnection, PhantomData<&'a ()>, @@ -88,3 +94,15 @@ impl<'a> std::ops::Deref for DerefConnection<'a> { } } } + +#[derive(Debug, failure::Fail)] +pub enum PoolError { + #[fail(display = "the provided database URL was not valid")] + InvalidDatabaseUrl(#[fail(cause)] postgres::Error), + + #[fail(display = "failed to create the connection pool")] + PoolCreationFailed(#[fail(cause)] r2d2::Error), + + #[fail(display = "failed to get a database connection")] + ConnectionError(#[fail(cause)] r2d2::Error), +} diff --git a/src/docbuilder/mod.rs b/src/docbuilder/mod.rs index 2b48f585e..240b2f86c 100644 --- a/src/docbuilder/mod.rs +++ b/src/docbuilder/mod.rs @@ -10,6 +10,7 @@ pub(self) use self::metadata::Metadata; pub(crate) use self::rustwide_builder::BuildResult; pub use self::rustwide_builder::RustwideBuilder; +use crate::db::Pool; use crate::error::Result; use crate::index::Index; use crate::DocBuilderOptions; @@ -24,16 +25,18 @@ use std::path::PathBuf; pub struct DocBuilder { options: DocBuilderOptions, index: Index, + db: Pool, cache: BTreeSet, db_cache: BTreeSet, } impl DocBuilder { - pub fn new(options: DocBuilderOptions) -> DocBuilder { + pub fn new(options: DocBuilderOptions, db: Pool) -> DocBuilder { let index = Index::new(&options.registry_index_path).expect("valid index"); DocBuilder { options, index, + db, cache: BTreeSet::new(), db_cache: BTreeSet::new(), } @@ -60,9 +63,7 @@ impl DocBuilder { fn load_database_cache(&mut self) -> Result<()> { debug!("Loading database cache"); - use crate::db::connect_db; - let conn = connect_db()?; - + let conn = self.db.get()?; for row in &conn.query( "SELECT name, version FROM crates, releases \ WHERE crates.id = releases.crate_id", diff --git a/src/docbuilder/queue.rs b/src/docbuilder/queue.rs index 54cfc936a..559425f14 100644 --- a/src/docbuilder/queue.rs +++ b/src/docbuilder/queue.rs @@ -1,7 +1,6 @@ //! Updates registry index and builds new packages use super::{DocBuilder, RustwideBuilder}; -use crate::db::connect_db; use crate::error::Result; use crate::utils::{add_crate_to_queue, get_crate_priority}; use crates_index_diff::ChangeKind; @@ -11,7 +10,7 @@ impl DocBuilder { /// Updates registry index repository and adds new crates into build queue. /// Returns the number of crates added pub fn get_new_crates(&mut self) -> Result { - let conn = connect_db()?; + let conn = self.db.get()?; let diff = self.index.diff()?; let (mut changes, oid) = diff.peek_changes()?; let mut crates_added = 0; @@ -65,7 +64,7 @@ impl DocBuilder { } pub fn get_queue_count(&self) -> Result { - let conn = connect_db()?; + let conn = self.db.get()?; Ok(conn .query("SELECT COUNT(*) FROM queue WHERE attempt < 5", &[])? @@ -78,32 +77,39 @@ impl DocBuilder { &mut self, builder: &mut RustwideBuilder, ) -> Result { - let conn = connect_db()?; - - let query = conn.query( - "SELECT id, name, version - FROM queue - WHERE attempt < 5 - ORDER BY priority ASC, attempt ASC, id ASC - LIMIT 1", - &[], - )?; - - if query.is_empty() { - // nothing in the queue; bail - return Ok(false); - } + // This is in a nested scope to drop the connection before build_package is called, + // otherwise the borrow checker will complain. + let (id, name, version): (i64, String, String) = { + let conn = self.db.get()?; + + let query = conn.query( + "SELECT id, name, version + FROM queue + WHERE attempt < 5 + ORDER BY priority ASC, attempt ASC, id ASC + LIMIT 1", + &[], + )?; + + if query.is_empty() { + // nothing in the queue; bail + return Ok(false); + } - let id: i32 = query.get(0).get(0); - let name: String = query.get(0).get(1); - let version: String = query.get(0).get(2); + let row = query.get(0); + (row.get("id"), row.get("name"), row.get("version")) + }; match builder.build_package(self, &name, &version, None) { Ok(_) => { + let conn = self.db.get()?; + let _ = conn.execute("DELETE FROM queue WHERE id = $1", &[&id]); crate::web::metrics::TOTAL_BUILDS.inc(); } Err(e) => { + let conn = self.db.get()?; + // Increase attempt count let rows = conn.query( "UPDATE queue SET attempt = attempt + 1 WHERE id = $1 RETURNING attempt", @@ -127,23 +133,3 @@ impl DocBuilder { Ok(true) } } - -#[cfg(test)] -mod test { - use crate::{DocBuilder, DocBuilderOptions}; - use log::error; - use std::path::PathBuf; - - #[test] - #[ignore] - fn test_get_new_crates() { - crate::test::init_logger(); - let options = DocBuilderOptions::from_prefix(PathBuf::from("../cratesfyi-prefix")); - let mut docbuilder = DocBuilder::new(options); - let res = docbuilder.get_new_crates(); - if res.is_err() { - error!("{:?}", res); - } - assert!(res.is_ok()); - } -} diff --git a/src/docbuilder/rustwide_builder.rs b/src/docbuilder/rustwide_builder.rs index 667afa982..0a7b36c7c 100644 --- a/src/docbuilder/rustwide_builder.rs +++ b/src/docbuilder/rustwide_builder.rs @@ -2,7 +2,7 @@ use super::DocBuilder; use super::Metadata; use crate::db::blacklist::is_blacklisted; use crate::db::file::add_path_into_database; -use crate::db::{add_build_into_database, add_package_into_database, connect_db}; +use crate::db::{add_build_into_database, add_package_into_database, Pool}; use crate::docbuilder::{crates::crates_from_path, Limits}; use crate::error::Result; use crate::storage::CompressionAlgorithms; @@ -66,12 +66,13 @@ const DUMMY_CRATE_VERSION: &str = "1.0.0"; pub struct RustwideBuilder { workspace: Workspace, toolchain: Toolchain, + db: Pool, rustc_version: String, cpu_limit: Option, } impl RustwideBuilder { - pub fn init() -> Result { + pub fn init(db: Pool) -> Result { use rustwide::cmd::SandboxImage; let env_workspace_path = ::std::env::var("CRATESFYI_RUSTWIDE_WORKSPACE"); let workspace_path = env_workspace_path @@ -105,6 +106,7 @@ impl RustwideBuilder { Ok(RustwideBuilder { workspace, toolchain, + db, rustc_version: String::new(), cpu_limit, }) @@ -192,7 +194,7 @@ impl RustwideBuilder { info!("building a dummy crate to get essential files"); - let conn = connect_db()?; + let conn = self.db.get()?; let limits = Limits::for_crate(&conn, DUMMY_CRATE_NAME)?; let mut build_dir = self @@ -305,7 +307,7 @@ impl RustwideBuilder { info!("building package {} {}", name, version); - let conn = connect_db()?; + let conn = self.db.get()?; if is_blacklisted(&conn, name)? { info!("skipping build of {}, crate has been blacklisted", name); diff --git a/src/test/mod.rs b/src/test/mod.rs index 5085e44b4..21e9152aa 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -143,7 +143,7 @@ impl TestEnvironment { pub(crate) fn db(&self) -> &TestDatabase { self.db - .get_or_init(|| TestDatabase::new().expect("failed to initialize the db")) + .get_or_init(|| TestDatabase::new(&self.config()).expect("failed to initialize the db")) } pub(crate) fn frontend(&self) -> &TestFrontend { @@ -162,12 +162,12 @@ pub(crate) struct TestDatabase { } impl TestDatabase { - fn new() -> Result { + fn new(config: &Config) -> Result { // A random schema name is generated and used for the current connection. This allows each // test to create a fresh instance of the database to run within. let schema = format!("docs_rs_test_schema_{}", rand::random::()); - let conn = crate::db::connect_db()?; + let conn = Connection::connect(config.database_url.as_str(), postgres::TlsMode::None)?; conn.batch_execute(&format!( " CREATE SCHEMA {0}; diff --git a/src/utils/daemon.rs b/src/utils/daemon.rs index 82c56b353..48ba449c5 100644 --- a/src/utils/daemon.rs +++ b/src/utils/daemon.rs @@ -3,6 +3,7 @@ //! This daemon will start web server, track new packages and build them use crate::{ + db::Pool, docbuilder::RustwideBuilder, utils::{github_updater, pubsubhubbub, update_release_activity}, Config, DocBuilder, DocBuilderOptions, @@ -16,7 +17,7 @@ use std::sync::Arc; use std::time::Duration; use std::{env, thread}; -pub fn start_daemon(config: Arc) -> Result<(), Error> { +pub fn start_daemon(config: Arc, db: Pool) -> Result<(), Error> { const CRATE_VARIABLES: [&str; 3] = [ "CRATESFYI_PREFIX", "CRATESFYI_GITHUB_USERNAME", @@ -36,6 +37,7 @@ pub fn start_daemon(config: Arc) -> Result<(), Error> { dbopts.check_paths().unwrap(); // check new crates every minute + let cloned_db = db.clone(); thread::Builder::new() .name("registry index reader".to_string()) .spawn(move || { @@ -43,7 +45,7 @@ pub fn start_daemon(config: Arc) -> Result<(), Error> { thread::sleep(Duration::from_secs(30)); loop { let opts = opts(); - let mut doc_builder = DocBuilder::new(opts); + let mut doc_builder = DocBuilder::new(opts, cloned_db.clone()); if doc_builder.is_locked() { debug!("Lock file exists, skipping checking new crates"); @@ -62,9 +64,10 @@ pub fn start_daemon(config: Arc) -> Result<(), Error> { // build new crates every minute // REFACTOR: Break this into smaller functions + let cloned_db = db.clone(); thread::Builder::new().name("build queue reader".to_string()).spawn(move || { let opts = opts(); - let mut doc_builder = DocBuilder::new(opts); + let mut doc_builder = DocBuilder::new(opts, cloned_db.clone()); /// Represents the current state of the builder thread. enum BuilderState { @@ -79,7 +82,7 @@ pub fn start_daemon(config: Arc) -> Result<(), Error> { QueueInProgress(usize), } - let mut builder = RustwideBuilder::init().unwrap(); + let mut builder = RustwideBuilder::init(cloned_db).unwrap(); let mut status = BuilderState::Fresh; @@ -192,38 +195,52 @@ pub fn start_daemon(config: Arc) -> Result<(), Error> { }).unwrap(); // update release activity everyday at 23:55 - thread::Builder::new() - .name("release activity updater".to_string()) - .spawn(move || loop { - thread::sleep(Duration::from_secs(60)); + let cloned_db = db.clone(); + cron( + "release activity updater", + Duration::from_secs(60), + move || { let now = Utc::now(); - if now.hour() == 23 && now.minute() == 55 { info!("Updating release activity"); - if let Err(e) = update_release_activity() { - error!("Failed to update release activity: {}", e); - } + update_release_activity(&*cloned_db.get()?)?; } - }) - .unwrap(); + Ok(()) + }, + )?; // update github stats every 6 hours - thread::Builder::new() - .name("github stat updater".to_string()) - .spawn(move || loop { - thread::sleep(Duration::from_secs(60 * 60 * 6)); - if let Err(e) = github_updater() { - error!("Failed to update github fields: {}", e); - } - }) - .unwrap(); + let cloned_db = db.clone(); + cron( + "github stats updater", + Duration::from_secs(60 * 60 * 6), + move || { + github_updater(&*cloned_db.get()?)?; + Ok(()) + }, + )?; // TODO: update ssl certificate every 3 months // at least start web server info!("Starting web server"); - crate::Server::start(None, false, config)?; + crate::Server::start(None, false, db, config)?; + Ok(()) +} + +fn cron(name: &'static str, interval: Duration, exec: F) -> Result<(), Error> +where + F: Fn() -> Result<(), Error> + Send + 'static, +{ + thread::Builder::new() + .name(name.into()) + .spawn(move || loop { + thread::sleep(interval); + if let Err(err) = exec() { + error!("failed to run scheduled task '{}': {:?}", name, err); + } + })?; Ok(()) } diff --git a/src/utils/github_updater.rs b/src/utils/github_updater.rs index fba5a8ebb..941a315f8 100644 --- a/src/utils/github_updater.rs +++ b/src/utils/github_updater.rs @@ -1,7 +1,8 @@ -use crate::{db::connect_db, error::Result}; +use crate::error::Result; use chrono::{DateTime, Utc}; use failure::err_msg; use log::debug; +use postgres::Connection; use regex::Regex; use std::str::FromStr; @@ -16,9 +17,7 @@ struct GitHubFields { } /// Updates github fields in crates table -pub fn github_updater() -> Result<()> { - let conn = connect_db()?; - +pub fn github_updater(conn: &Connection) -> Result<()> { // TODO: This query assumes repository field in Cargo.toml is // always the same across all versions of a crate for row in &conn.query( @@ -175,26 +174,4 @@ mod test { Some("docopt/docopt.rs".to_string()) ); } - - #[test] - #[ignore] - fn test_get_github_fields() { - let fields = get_github_fields("onur/cratesfyi"); - assert!(fields.is_ok()); - - let fields = fields.unwrap(); - assert!(fields.description != ""); - assert!(fields.stars >= 0); - assert!(fields.forks >= 0); - assert!(fields.issues >= 0); - - assert!(fields.last_commit <= Utc::now()); - } - - #[test] - #[ignore] - fn test_github_updater() { - crate::test::init_logger(); - assert!(github_updater().is_ok()); - } } diff --git a/src/utils/release_activity_updater.rs b/src/utils/release_activity_updater.rs index ffe92f40a..71e0032bc 100644 --- a/src/utils/release_activity_updater.rs +++ b/src/utils/release_activity_updater.rs @@ -1,10 +1,9 @@ -use crate::db::connect_db; use crate::error::Result; use chrono::{Duration, Utc}; +use postgres::Connection; use serde_json::{Map, Value}; -pub fn update_release_activity() -> Result<()> { - let conn = connect_db()?; +pub fn update_release_activity(conn: &Connection) -> Result<()> { let mut dates = Vec::with_capacity(30); let mut crate_counts = Vec::with_capacity(30); let mut failure_counts = Vec::with_capacity(30); @@ -67,15 +66,3 @@ pub fn update_release_activity() -> Result<()> { Ok(()) } - -#[cfg(test)] -mod test { - use super::update_release_activity; - - #[test] - #[ignore] - fn test_update_release_activity() { - crate::test::init_logger(); - assert!(update_release_activity().is_ok()); - } -} diff --git a/src/web/builds.rs b/src/web/builds.rs index f6186473f..1e0fe9934 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -1,7 +1,7 @@ use super::duration_to_str; use super::page::Page; -use super::pool::Pool; use super::MetaData; +use crate::db::Pool; use crate::docbuilder::Limits; use chrono::{DateTime, NaiveDateTime, Utc}; use iron::prelude::*; diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 273bc4d96..c2baa84bf 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -1,9 +1,9 @@ use super::error::Nope; use super::page::Page; -use super::pool::Pool; use super::{ duration_to_str, match_version, redirect_base, render_markdown, MatchSemver, MetaData, }; +use crate::db::Pool; use chrono::{DateTime, NaiveDateTime, Utc}; use iron::prelude::*; use iron::{status, Url}; diff --git a/src/web/error.rs b/src/web/error.rs index 11cbab701..d49054ac3 100644 --- a/src/web/error.rs +++ b/src/web/error.rs @@ -1,4 +1,6 @@ +use crate::db::PoolError; use crate::web::page::Page; +use failure::Fail; use iron::prelude::*; use iron::status; use iron::Handler; @@ -71,3 +73,9 @@ impl Handler for Nope { } } } + +impl From for IronError { + fn from(err: PoolError) -> IronError { + IronError::new(err.compat(), status::InternalServerError) + } +} diff --git a/src/web/extensions.rs b/src/web/extensions.rs new file mode 100644 index 000000000..0054f72cd --- /dev/null +++ b/src/web/extensions.rs @@ -0,0 +1,35 @@ +use crate::config::Config; +use crate::db::Pool; +use crate::web::page::TemplateData; +use iron::{BeforeMiddleware, IronResult, Request}; +use std::sync::Arc; + +#[derive(Debug, Clone)] +pub(super) struct InjectExtensions { + pub(super) pool: Pool, + pub(super) config: Arc, + pub(super) template_data: Arc, +} + +impl BeforeMiddleware for InjectExtensions { + fn before(&self, req: &mut Request) -> IronResult<()> { + req.extensions.insert::(self.pool.clone()); + req.extensions.insert::(self.config.clone()); + req.extensions + .insert::(self.template_data.clone()); + + Ok(()) + } +} + +macro_rules! key { + ($key:ty => $value:ty) => { + impl iron::typemap::Key for $key { + type Value = $value; + } + }; +} + +key!(Pool => Pool); +key!(Config => Arc); +key!(TemplateData => Arc); diff --git a/src/web/file.rs b/src/web/file.rs index a2abbaeff..c94ce36d0 100644 --- a/src/web/file.rs +++ b/src/web/file.rs @@ -1,6 +1,6 @@ //! Database based file handler -use super::pool::Pool; +use crate::db::Pool; use crate::{db, error::Result, Config}; use iron::{status, Handler, IronError, IronResult, Request, Response}; use postgres::Connection; diff --git a/src/web/metrics.rs b/src/web/metrics.rs index cc7dac7f8..2cdca0976 100644 --- a/src/web/metrics.rs +++ b/src/web/metrics.rs @@ -1,4 +1,4 @@ -use super::pool::Pool; +use crate::db::Pool; use iron::headers::ContentType; use iron::prelude::*; use iron::status::Status; diff --git a/src/web/mod.rs b/src/web/mod.rs index 879e2f1a0..bb90defd6 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -46,25 +46,26 @@ macro_rules! extension { mod builds; mod crate_details; mod error; +mod extensions; mod file; pub(crate) mod metrics; -mod pool; mod releases; mod routes; mod rustdoc; mod sitemap; mod source; +use self::extensions::InjectExtensions; use self::page::TemplateData; -use self::pool::Pool; use crate::config::Config; +use crate::db::Pool; use chrono::{DateTime, Utc}; use failure::Error; use handlebars_iron::{DirectorySource, HandlebarsEngine, SourceError}; use iron::headers::{CacheControl, CacheDirective, ContentType, Expires, HttpDate}; use iron::modifiers::Redirect; use iron::prelude::*; -use iron::{self, status, BeforeMiddleware, Handler, Listening, Url}; +use iron::{self, status, Handler, Listening, Url}; use postgres::Connection; use router::NoRoute; use semver::{Version, VersionReq}; @@ -217,24 +218,6 @@ impl Handler for CratesfyiHandler { } } -#[derive(Debug, Clone)] -struct InjectExtensions { - pool: Pool, - config: Arc, - template_data: Arc, -} - -impl BeforeMiddleware for InjectExtensions { - fn before(&self, req: &mut Request) -> IronResult<()> { - req.extensions.insert::(self.pool.clone()); - req.extensions.insert::(self.config.clone()); - req.extensions - .insert::(self.template_data.clone()); - - Ok(()) - } -} - struct MatchVersion { /// Represents the crate name that was found when attempting to load a crate release. /// @@ -395,20 +378,16 @@ impl Server { pub fn start( addr: Option<&str>, reload_templates: bool, + db: Pool, config: Arc, ) -> Result { // Initialize templates - let template_data = Arc::new(TemplateData::new()?); + let template_data = Arc::new(TemplateData::new(&*db.get()?)?); if reload_templates { - TemplateData::start_template_reloading(template_data.clone()); + TemplateData::start_template_reloading(template_data.clone(), db.clone()); } - let server = Self::start_inner( - addr.unwrap_or(DEFAULT_BIND), - Pool::new(), - config, - template_data, - ); + let server = Self::start_inner(addr.unwrap_or(DEFAULT_BIND), db, config, template_data); info!("Running docs.rs web server on http://{}", server.addr()); Ok(server) } @@ -418,11 +397,13 @@ impl Server { conn: Arc>, config: Arc, ) -> Result { + let templates = TemplateData::new(&conn.lock().unwrap())?; + Ok(Self::start_inner( "127.0.0.1:0", Pool::new_simple(conn.clone()), config, - Arc::new(TemplateData::new()?), + Arc::new(templates), )) } diff --git a/src/web/page/handlebars.rs b/src/web/page/handlebars.rs index 05a10c413..8755fd089 100644 --- a/src/web/page/handlebars.rs +++ b/src/web/page/handlebars.rs @@ -16,7 +16,14 @@ lazy_static::lazy_static! { } fn load_rustc_resource_suffix() -> Result { - let conn = crate::db::connect_db()?; + // New instances of the configuration or the connection pool shouldn't be created inside the + // application, but we're removing handlebars so this is not going to be a problem in the long + // term. To avoid wasting resources, the pool is hardcoded to only keep one connection alive. + let mut config = crate::Config::from_env()?; + config.max_pool_size = 1; + config.min_pool_idle = 1; + let pool = crate::db::Pool::new(&config)?; + let conn = pool.get()?; let res = conn.query( "SELECT value FROM config WHERE name = 'rustc_version';", diff --git a/src/web/page/templates.rs b/src/web/page/templates.rs index 4d1872474..3599c3d93 100644 --- a/src/web/page/templates.rs +++ b/src/web/page/templates.rs @@ -1,7 +1,9 @@ +use crate::db::Pool; use crate::error::Result; use arc_swap::ArcSwap; use chrono::{DateTime, Utc}; use notify::{watcher, RecursiveMode, Watcher}; +use postgres::Connection; use serde_json::Value; use std::collections::HashMap; use std::sync::{mpsc::channel, Arc}; @@ -18,11 +20,11 @@ pub(crate) struct TemplateData { } impl TemplateData { - pub(crate) fn new() -> Result { + pub(crate) fn new(conn: &Connection) -> Result { log::trace!("Loading templates"); let data = Self { - templates: ArcSwap::from_pointee(load_templates()?), + templates: ArcSwap::from_pointee(load_templates(conn)?), }; log::trace!("Finished loading templates"); @@ -30,7 +32,7 @@ impl TemplateData { Ok(data) } - pub(crate) fn start_template_reloading(template_data: Arc) { + pub(crate) fn start_template_reloading(template_data: Arc, pool: Pool) { let (tx, rx) = channel(); // Set a 2 second event debounce for the watcher let mut watcher = watcher(tx, Duration::from_secs(2)).unwrap(); @@ -40,33 +42,30 @@ impl TemplateData { .unwrap(); thread::spawn(move || { + fn reload(template_data: &TemplateData, pool: &Pool) -> Result<()> { + let conn = pool.get()?; + template_data + .templates + .swap(Arc::new(load_templates(&conn)?)); + log::info!("Reloaded templates"); + + Ok(()) + } + // The watcher needs to be moved into the thread so that it's not dropped (when dropped, // all updates cease) let _watcher = watcher; while rx.recv().is_ok() { - match load_templates() { - Ok(templates) => { - log::info!("Reloaded templates"); - template_data.templates.swap(Arc::new(templates)); - } - - Err(err) => log::error!("Error reloading templates: {:?}", err), + if let Err(err) = reload(&template_data, &pool) { + log::error!("failed to reload templates: {:?}", err); } } }); } } -impl iron::typemap::Key for TemplateData { - type Value = std::sync::Arc; -} - -// TODO: Is there a reason this isn't fatal? If the rustc version is incorrect (Or "???" as used by default), then -// all pages will be served *really* weird because they'll lack all CSS -fn load_rustc_resource_suffix() -> Result { - let conn = crate::db::connect_db()?; - +fn load_rustc_resource_suffix(conn: &Connection) -> Result { let res = conn.query( "SELECT value FROM config WHERE name = 'rustc_version';", &[], @@ -84,7 +83,7 @@ fn load_rustc_resource_suffix() -> Result { failure::bail!("failed to parse the rustc version"); } -pub(super) fn load_templates() -> Result { +pub(super) fn load_templates(conn: &Connection) -> Result { let mut tera = Tera::new("tera-templates/**/*")?; // This function will return any global alert, if present. @@ -104,8 +103,11 @@ pub(super) fn load_templates() -> Result { ReturnValue::add_function_to( &mut tera, "rustc_resource_suffix", - Value::String(load_rustc_resource_suffix().unwrap_or_else(|err| { + Value::String(load_rustc_resource_suffix(conn).unwrap_or_else(|err| { log::error!("Failed to load rustc resource suffix: {:?}", err); + // This is not fatal because the server might be started before essential files are + // generated during development. Returning "???" provides a degraded UX, but allows the + // server to start every time. String::from("???") })), ); @@ -199,7 +201,13 @@ mod tests { #[test] fn test_templates_are_valid() { - let tera = load_templates().unwrap(); - tera.check_macro_files().unwrap(); + crate::test::wrapper(|env| { + let db = env.db(); + + let tera = load_templates(&db.conn()).unwrap(); + tera.check_macro_files().unwrap(); + + Ok(()) + }); } } diff --git a/src/web/releases.rs b/src/web/releases.rs index 777c1e770..15464dd9e 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -2,8 +2,8 @@ use super::error::Nope; use super::page::Page; -use super::pool::Pool; use super::{duration_to_str, match_version, redirect_base}; +use crate::db::Pool; use chrono::{DateTime, NaiveDateTime, Utc}; use iron::prelude::*; use iron::status; diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index 5374a02a7..537422509 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -5,9 +5,9 @@ use super::error::Nope; use super::file::File; use super::metrics; use super::page::Page; -use super::pool::Pool; use super::redirect_base; use super::{match_version, MatchSemver}; +use crate::db::Pool; use crate::utils; use crate::Config; use iron::headers::{CacheControl, CacheDirective, Expires, HttpDate}; diff --git a/src/web/sitemap.rs b/src/web/sitemap.rs index 4abc35f0a..73068c318 100644 --- a/src/web/sitemap.rs +++ b/src/web/sitemap.rs @@ -1,8 +1,5 @@ -use crate::{ - docbuilder::Limits, - impl_webpage, - web::{page::WebPage, pool::Pool}, -}; +use crate::db::Pool; +use crate::{docbuilder::Limits, impl_webpage, web::page::WebPage}; use chrono::{DateTime, NaiveDateTime, Utc}; use iron::{ headers::ContentType, diff --git a/src/web/source.rs b/src/web/source.rs index ba8d47657..49bca35e1 100644 --- a/src/web/source.rs +++ b/src/web/source.rs @@ -2,8 +2,8 @@ use super::file::File as DbFile; use super::page::Page; -use super::pool::Pool; use super::MetaData; +use crate::db::Pool; use crate::Config; use iron::prelude::*; use postgres::Connection;