Merge remote-tracking branch 'upstream/master' into update-itertools-log

Author: carols10cents

.diesel_version

@@ -1 +1 @@
-1.3.0
+1.4.0

.travis.yml

@@ -30,7 +30,7 @@ install:
   - cargo install --force diesel_cli --vers `cat .diesel_version` --no-default-features --features postgres && export PATH=$HOME/.cargo/bin:$PATH
 
 before_script:
-  - diesel database setup
+  - diesel database setup --locked-schema
 
 addons:
   chrome: stable

Cargo.lock

@@ -47,7 +47,7 @@ htmlescape = "0.3.1"
 license-exprs = "^1.4"
 dotenv = "0.11.0"
 toml = "0.4"
-diesel = { version = "1.3.0", features = ["postgres", "serde_json", "chrono", "r2d2"] }
+diesel = { version = "1.4.0", features = ["postgres", "serde_json", "chrono", "r2d2"] }
 diesel_full_text_search = "1.0.0"
 diesel_ltree = "0.1.3"
 serde_json = "1.0.0"
@@ -62,14 +62,15 @@ scheduled-thread-pool = "0.2.0"
 derive_deref = "1.0.0"
 reqwest = "0.9.1"
 tempdir = "0.3.7"
+parking_lot = "0.7.1"
 
 lettre = {git = "https://github.com/lettre/lettre", version = "0.9"}
 lettre_email = {git = "https://github.com/lettre/lettre", version = "0.9"}
 
 conduit = "0.8"
 conduit-conditional-get = "0.8"
-conduit-cookie = "0.8"
-cookie = "0.9"
+conduit-cookie = "0.8.5"
+cookie = "0.11"
 conduit-middleware = "0.8"
 conduit-router = "0.8"
 conduit-static = "0.8"
@@ -89,7 +90,3 @@ tokio-service = "0.1"
 dotenv = "0.11"
 diesel = { version = "1.3.0", features = ["postgres"] }
 diesel_migrations = { version = "1.3.0", features = ["postgres"] }
-
-# Remove once cookie depends on ring >= 0.13.0
-[patch.crates-io]
-ring = { git = "https://github.com/SergioBenitez/ring", branch = "v0.11" }

Cargo.toml

@@ -1,2 +1,2 @@
-web: bin/diesel migration run && bin/start-nginx ./target/release/server
+web: bin/diesel migration run --locked-schema && bin/start-nginx ./target/release/server
 worker: ./target/release/update-downloads daemon 300

Procfile

@@ -73,6 +73,7 @@ impl App {
         let db_connection_timeout = match (env::var("DB_TIMEOUT"), config.env) {
             (Ok(num), _) => num.parse().expect("couldn't parse DB_TIMEOUT"),
             (_, Env::Production) => 10,
+            (_, Env::Test) => 1,
             _ => 30,
         };
 
@@ -88,7 +89,7 @@ impl App {
         let repo = git2::Repository::open(&config.git_repo_checkout).unwrap();
 
         App {
-            diesel_database: db::diesel_pool(&config.db_url, diesel_db_config),
+            diesel_database: db::diesel_pool(&config.db_url, config.env, diesel_db_config),
             github,
             session_key: config.session_key.clone(),
             git_repo: Mutex::new(repo),

src/app.rs

@@ -1,5 +1,4 @@
 #![deny(warnings)]
-#![allow(unknown_lints, proc_macro_derive_resolution_fallback)] // This can be removed after diesel-1.4
 
 #[macro_use]
 extern crate diesel;
@@ -138,7 +137,7 @@ mod test {
             name: "foo",
             ..Default::default()
         }
-        .create_or_update(&conn, None, user_id)
+        .create_or_update(conn, None, user_id)
         .unwrap();
         let version = NewVersion::new(
             krate.id,
@@ -149,7 +148,7 @@ mod test {
             0,
         )
         .unwrap();
-        let version = version.save(&conn, &[]).unwrap();
+        let version = version.save(conn, &[]).unwrap();
         (krate, version)
     }
 

src/bin/update-downloads.rs

@@ -32,11 +32,11 @@ struct OwnerInvitation {
 
 /// Handles the `PUT /me/crate_owner_invitations/:crate_id` route.
 pub fn handle_invite(req: &mut dyn Request) -> CargoResult<Response> {
-    let conn = &*req.db_conn()?;
-
     let mut body = String::new();
     req.body().read_to_string(&mut body)?;
 
+    let conn = &*req.db_conn()?;
+
     let crate_invite: OwnerInvitation =
         serde_json::from_str(&body).map_err(|_| human("invalid json request"))?;
 
@@ -50,7 +50,7 @@ pub fn handle_invite(req: &mut dyn Request) -> CargoResult<Response> {
 }
 
 fn accept_invite(
-    req: &mut dyn Request,
+    req: &dyn Request,
     conn: &PgConnection,
     crate_invite: InvitationResponse,
 ) -> CargoResult<Response> {
@@ -88,7 +88,7 @@ fn accept_invite(
 }
 
 fn decline_invite(
-    req: &mut dyn Request,
+    req: &dyn Request,
     conn: &PgConnection,
     crate_invite: InvitationResponse,
 ) -> CargoResult<Response> {

src/controllers/crate_owner_invitation.rs

@@ -63,7 +63,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {
     let categories = new_crate.categories.as_ref().map(|s| &s[..]).unwrap_or(&[]);
     let categories: Vec<_> = categories.iter().map(|k| &***k).collect();
 
-    let conn = req.db_conn()?;
+    let conn = app.diesel_database.get()?;
 
     let mut other_warnings = vec![];
     if !user.has_verified_email(&conn)? {

src/controllers/krate/publish.rs

@@ -171,7 +171,7 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
 
     let badges = CrateBadge::belonging_to(&crates)
         .select((badges::crate_id, badges::all_columns))
-        .load::<CrateBadge>(&conn)?
+        .load::<CrateBadge>(&*conn)?
         .grouped_by(&crates)
         .into_iter()
         .map(|badges| badges.into_iter().map(|cb| cb.badge).collect());

src/controllers/krate/search.rs

@@ -39,7 +39,7 @@ mod prelude {
 
         fn redirect(&self, url: String) -> Response {
             let mut headers = HashMap::new();
-            headers.insert("Location".to_string(), vec![url.to_string()]);
+            headers.insert("Location".to_string(), vec![url]);
             Response {
                 status: (302, "Found"),
                 headers,

src/controllers/mod.rs

@@ -94,11 +94,7 @@ pub fn github_access_token(req: &mut dyn Request) -> CargoResult<Response> {
     }
 
     // Fetch the access token from github using the code we just got
-    let token = req
-        .app()
-        .github
-        .exchange(code.clone())
-        .map_err(|s| human(&s))?;
+    let token = req.app().github.exchange(code).map_err(|s| human(&s))?;
 
     let ghuser = github::github::<GithubUser>(req.app(), "/user", &token)?;
 

src/controllers/user/session.rs

@@ -7,12 +7,10 @@
 
 use crate::controllers::prelude::*;
 
-use crate::models::Version;
+use crate::models::{Crate, Version};
 use crate::schema::*;
 use crate::views::EncodableVersion;
 
-use super::version_and_crate;
-
 /// Handles the `GET /versions` route.
 pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
     use diesel::dsl::any;
@@ -40,28 +38,18 @@ pub fn index(req: &mut dyn Request) -> CargoResult<Response> {
     Ok(req.json(&R { versions }))
 }
 
-/// Handles the `GET /versions/:version_id` and
-/// `GET /crates/:crate_id/:version` routes.
-///
-/// The frontend doesn't appear to hit either of these endpoints. Instead the
-/// version information appears to be returned by `krate::show`.
-///
-/// FIXME: These two routes have very different semantics and should be split into
-/// a separate function for each endpoint.
-pub fn show(req: &mut dyn Request) -> CargoResult<Response> {
-    let (version, krate) = match req.params().find("crate_id") {
-        Some(..) => version_and_crate(req)?,
-        None => {
-            let id = &req.params()["version_id"];
-            let id = id.parse().unwrap_or(0);
-            let conn = req.db_conn()?;
-            versions::table
-                .find(id)
-                .inner_join(crates::table)
-                .select((versions::all_columns, crate::models::krate::ALL_COLUMNS))
-                .first(&*conn)?
-        }
-    };
+/// Handles the `GET /versions/:version_id` route.
+/// The frontend doesn't appear to hit this endpoint. Instead, the version information appears to
+/// be returned by `krate::show`.
+pub fn show_by_id(req: &mut dyn Request) -> CargoResult<Response> {
+    let id = &req.params()["version_id"];
+    let id = id.parse().unwrap_or(0);
+    let conn = req.db_conn()?;
+    let (version, krate): (Version, Crate) = versions::table
+        .find(id)
+        .inner_join(crates::table)
+        .select((versions::all_columns, crate::models::krate::ALL_COLUMNS))
+        .first(&*conn)?;
 
     #[derive(Serialize)]
     struct R {

src/controllers/version/deprecated.rs

@@ -7,7 +7,7 @@
 use crate::controllers::prelude::*;
 
 use crate::schema::*;
-use crate::views::{EncodableDependency, EncodablePublicUser};
+use crate::views::{EncodableDependency, EncodablePublicUser, EncodableVersion};
 
 use super::version_and_crate;
 
@@ -61,3 +61,19 @@ pub fn authors(req: &mut dyn Request) -> CargoResult<Response> {
         meta: Meta { names },
     }))
 }
+
+/// Handles the `GET /crates/:crate/:version` route.
+///
+/// The frontend doesn't appear to hit this endpoint, but our tests do, and it seems to be a useful
+/// API route to have.
+pub fn show(req: &mut dyn Request) -> CargoResult<Response> {
+    let (version, krate) = version_and_crate(req)?;
+
+    #[derive(Serialize)]
+    struct R {
+        version: EncodableVersion,
+    }
+    Ok(req.json(&R {
+        version: version.encodable(&krate.name),
+    }))
+}

src/controllers/version/metadata.rs

@@ -3,13 +3,56 @@ use std::env;
 use conduit::Request;
 use diesel::prelude::*;
 use diesel::r2d2::{self, ConnectionManager, CustomizeConnection};
+use parking_lot::{ReentrantMutex, ReentrantMutexGuard};
+use std::ops::Deref;
 use url::Url;
 
 use crate::middleware::app::RequestApp;
 use crate::util::CargoResult;
+use crate::Env;
 
-pub type DieselPool = r2d2::Pool<ConnectionManager<PgConnection>>;
-type DieselPooledConn = r2d2::PooledConnection<ConnectionManager<PgConnection>>;
+#[allow(missing_debug_implementations)]
+pub enum DieselPool {
+    Pool(r2d2::Pool<ConnectionManager<PgConnection>>),
+    Test(ReentrantMutex<PgConnection>),
+}
+
+impl DieselPool {
+    pub fn get(&self) -> CargoResult<DieselPooledConn> {
+        match self {
+            DieselPool::Pool(pool) => Ok(DieselPooledConn::Pool(pool.get()?)),
+            DieselPool::Test(conn) => Ok(DieselPooledConn::Test(conn.lock())),
+        }
+    }
+
+    pub fn state(&self) -> r2d2::State {
+        match self {
+            DieselPool::Pool(pool) => pool.state(),
+            DieselPool::Test(_) => panic!("Cannot get the state of a test pool"),
+        }
+    }
+
+    fn test_conn(conn: PgConnection) -> Self {
+        DieselPool::Test(ReentrantMutex::new(conn))
+    }
+}
+
+#[allow(missing_debug_implementations)]
+pub enum DieselPooledConn<'a> {
+    Pool(r2d2::PooledConnection<ConnectionManager<PgConnection>>),
+    Test(ReentrantMutexGuard<'a, PgConnection>),
+}
+
+impl Deref for DieselPooledConn<'_> {
+    type Target = PgConnection;
+
+    fn deref(&self) -> &Self::Target {
+        match self {
+            DieselPooledConn::Pool(conn) => conn.deref(),
+            DieselPooledConn::Test(conn) => conn.deref(),
+        }
+    }
+}
 
 pub fn connect_now() -> ConnectionResult<PgConnection> {
     use diesel::Connection;
@@ -22,14 +65,22 @@ pub fn connect_now() -> ConnectionResult<PgConnection> {
 
 pub fn diesel_pool(
     url: &str,
+    env: Env,
     config: r2d2::Builder<ConnectionManager<PgConnection>>,
 ) -> DieselPool {
     let mut url = Url::parse(url).expect("Invalid database URL");
     if env::var("HEROKU").is_ok() && !url.query_pairs().any(|(k, _)| k == "sslmode") {
         url.query_pairs_mut().append_pair("sslmode", "require");
     }
-    let manager = ConnectionManager::new(url.into_string());
-    config.build(manager).unwrap()
+
+    if env == Env::Test {
+        let conn =
+            PgConnection::establish(&url.into_string()).expect("failed to establish connection");
+        DieselPool::test_conn(conn)
+    } else {
+        let manager = ConnectionManager::new(url.into_string());
+        DieselPool::Pool(config.build(manager).unwrap())
+    }
 }
 
 pub trait RequestTransaction {

src/db.rs

@@ -7,7 +7,6 @@
 #![deny(missing_debug_implementations, missing_copy_implementations)]
 #![deny(bare_trait_objects)]
 #![recursion_limit = "256"]
-#![allow(unknown_lints, proc_macro_derive_resolution_fallback)] // TODO: This can be removed after diesel-1.4
 
 #[macro_use]
 extern crate derive_deref;

src/lib.rs

@@ -20,6 +20,8 @@ pub enum AuthenticationSource {
 
 impl Middleware for CurrentUser {
     fn before(&self, req: &mut dyn Request) -> Result<(), Box<dyn Error + Send>> {
+        use std::mem::drop;
+
         // Check if the request has a session cookie with a `user_id` property inside
         let id = {
             req.session()
@@ -32,6 +34,7 @@ impl Middleware for CurrentUser {
         if let Some(id) = id {
             // If it did, look for a user in the database with the given `user_id`
             let maybe_user = users::table.find(id).first::<User>(&*conn);
+            drop(conn);
             if let Ok(user) = maybe_user {
                 // Attach the `User` model from the database to the request
                 req.mut_extensions().insert(user);
@@ -46,6 +49,7 @@ impl Middleware for CurrentUser {
             } else {
                 None
             };
+            drop(conn);
             if let Some(user) = user {
                 // Attach the `User` model from the database to the request
                 req.mut_extensions().insert(user);