Port crates#summary over to use Diesel

This endpoint was really contained, so very little supporting code had
to change. This endpoint is extremely light on test coverage, so it
should be verified on staging. As a bonus, it no longer has an N+1
queries bug, and will execute 8 queries instead of 35 (we can get that
down to 6 technically but I don't think it's worth it)

Author: sgrif

src/category.rs

@@ -8,11 +8,11 @@ use diesel::pg::PgConnection;
 use pg::GenericConnection;
 use pg::rows::Row;
 
-use {Model, Crate};
 use db::RequestTransaction;
 use schema::*;
-use util::{RequestUtils, CargoResult, ChainError};
 use util::errors::NotFound;
+use util::{RequestUtils, CargoResult, ChainError};
+use {Model, Crate};
 
 #[derive(Clone, Identifiable, Associations, Queryable)]
 #[has_many(crates_categories)]
@@ -181,7 +181,33 @@ impl Category {
         Ok(rows.iter().next().unwrap().get("count"))
     }
 
-    pub fn toplevel(conn: &GenericConnection,
+    pub fn toplevel(conn: &PgConnection,
+                    sort: &str,
+                    limit: i64,
+                    offset: i64) -> QueryResult<Vec<Category>> {
+        use diesel::select;
+        use diesel::expression::dsl::*;
+
+        let sort_sql = match sort {
+            "crates" => "ORDER BY crates_cnt DESC",
+            _ => "ORDER BY category ASC",
+        };
+
+        // Collect all the top-level categories and sum up the crates_cnt of
+        // the crates in all subcategories
+        select(sql::<categories::SqlType>(&format!(
+            "c.id, c.category, c.slug, c.description,
+                sum(c2.crates_cnt)::int as crates_cnt, c.created_at
+             FROM categories as c
+             INNER JOIN categories c2 ON split_part(c2.slug, '::', 1) = c.slug
+             WHERE split_part(c.slug, '::', 1) = c.slug
+             GROUP BY c.id
+             {} LIMIT {} OFFSET {}",
+            sort_sql, limit, offset
+        ))).load(conn)
+    }
+
+    pub fn toplevel_old(conn: &GenericConnection,
                     sort: &str,
                     limit: i64,
                     offset: i64) -> CargoResult<Vec<Category>> {
@@ -278,7 +304,7 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
     let query = req.query();
     let sort = query.get("sort").map_or("alpha", String::as_str);
 
-    let categories = Category::toplevel(conn, sort, limit, offset)?;
+    let categories = Category::toplevel_old(conn, sort, limit, offset)?;
     let categories = categories.into_iter().map(Category::encodable).collect();
 
     // Query for the total count of categories
@@ -342,15 +368,15 @@ pub fn slugs(req: &mut Request) -> CargoResult<Response> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use pg::{Connection, TlsMode};
+    use diesel::connection::SimpleConnection;
     use dotenv::dotenv;
     use std::env;
 
-    fn pg_connection() -> Connection {
+    fn pg_connection() -> PgConnection {
         let _ = dotenv();
         let database_url = env::var("TEST_DATABASE_URL")
             .expect("TEST_DATABASE_URL must be set to run tests");
-        let conn = Connection::connect(database_url, TlsMode::None).unwrap();
+        let conn = PgConnection::establish(&database_url).unwrap();
         // These tests deadlock if run concurrently
         conn.batch_execute("BEGIN; LOCK categories IN ACCESS EXCLUSIVE MODE").unwrap();
         conn

src/krate.rs

@@ -4,37 +4,36 @@ use std::collections::HashMap;
 
 use conduit::{Request, Response};
 use conduit_router::RequestParams;
-use diesel::prelude::*;
 use diesel::pg::PgConnection;
 use diesel::pg::upsert::*;
+use diesel::prelude::*;
 use diesel_full_text_search::*;
 use license_exprs;
 use pg::GenericConnection;
 use pg::rows::Row;
-use pg;
 use rustc_serialize::hex::ToHex;
 use rustc_serialize::json;
 use semver;
 use time::{Timespec, Duration};
 use url::Url;
 
-use {Model, User, Keyword, Version, Category, Badge, Replica};
 use app::{App, RequestApp};
+use badge::EncodableBadge;
+use category::EncodableCategory;
 use db::RequestTransaction;
 use dependency::{Dependency, EncodableDependency};
 use download::{VersionDownload, EncodableVersionDownload};
 use git;
 use keyword::EncodableKeyword;
-use category::EncodableCategory;
-use badge::EncodableBadge;
+use owner::{EncodableOwner, Owner, Rights, OwnerKind, Team, rights, CrateOwner};
+use schema::*;
 use upload;
 use user::RequestUser;
-use owner::{EncodableOwner, Owner, Rights, OwnerKind, Team, rights, CrateOwner};
 use util::errors::NotFound;
 use util::{read_le_u32, read_fill};
 use util::{RequestUtils, CargoResult, internal, ChainError, human};
 use version::EncodableVersion;
-use schema::*;
+use {Model, User, Keyword, Version, Category, Badge, Replica};
 
 #[derive(Clone, Queryable, Identifiable, AsChangeset)]
 pub struct Crate {
@@ -719,40 +718,50 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
 
 /// Handles the `GET /summary` route.
 pub fn summary(req: &mut Request) -> CargoResult<Response> {
-    let tx = req.tx()?;
-    let num_crates = Crate::count(tx)?;
-    let num_downloads = {
-        let stmt = tx.prepare("SELECT total_downloads FROM metadata")?;
-        let rows = stmt.query(&[])?;
-        rows.iter().next().unwrap().get("total_downloads")
-    };
+    use schema::crates::dsl::*;
 
-    let to_crates = |stmt: pg::stmt::Statement| -> CargoResult<Vec<_>> {
-        let rows = stmt.query(&[])?;
-        rows.iter().map(|r| {
-            let krate: Crate = Model::from_row(&r);
-            let max_version = krate.max_version(tx)?;
-            Ok(krate.minimal_encodable(max_version, None))
-        }).collect()
+    let conn = req.db_conn()?;
+    let num_crates = crates.count().get_result(conn)?;
+    let num_downloads = metadata::table.select(metadata::total_downloads)
+        .get_result(conn)?;
+
+    let encode_crates = |krates: Vec<Crate>| -> CargoResult<Vec<_>> {
+        Version::belonging_to(&krates)
+            .load::<Version>(conn)?
+            .grouped_by(&krates)
+            .into_iter()
+            .map(|versions| Version::max(versions.into_iter().map(|v| v.num)))
+            .zip(krates)
+            .map(|(max_version, krate)| {
+                 Ok(krate.minimal_encodable(max_version, None))
+            }).collect()
     };
-    let new_crates = tx.prepare("SELECT * FROM crates \
-                                        ORDER BY created_at DESC LIMIT 10")?;
-    let just_updated = tx.prepare("SELECT * FROM crates \
-                                        WHERE updated_at::timestamp(0) !=
-                                              created_at::timestamp(0)
-                                        ORDER BY updated_at DESC LIMIT 10")?;
-    let most_downloaded = tx.prepare("SELECT * FROM crates \
-                                           ORDER BY downloads DESC LIMIT 10")?;
-
-    let popular_keywords = Keyword::all(tx, "crates", 10, 0)?;
-    let popular_keywords = popular_keywords.into_iter()
-                                           .map(Keyword::encodable)
-                                           .collect();
-
-    let popular_categories = Category::toplevel(tx, "crates", 10, 0)?;
-    let popular_categories = popular_categories.into_iter()
-                                               .map(Category::encodable)
-                                               .collect();
+
+    let new_crates = crates.order(created_at.desc())
+        .select(ALL_COLUMNS)
+        .limit(10)
+        .load(conn)?;
+    let just_updated = crates.filter(updated_at.ne(created_at))
+        .order(updated_at.desc())
+        .select(ALL_COLUMNS)
+        .limit(10)
+        .load(conn)?;
+    let most_downloaded = crates.order(downloads.desc())
+        .select(ALL_COLUMNS)
+        .limit(10)
+        .load(conn)?;
+
+    let popular_keywords = keywords::table.order(keywords::crates_cnt.desc())
+        .limit(10)
+        .load(conn)?
+        .into_iter()
+        .map(Keyword::encodable)
+        .collect();
+
+    let popular_categories = Category::toplevel(conn, "crates", 10, 0)?
+        .into_iter()
+        .map(Category::encodable)
+        .collect();
 
     #[derive(RustcEncodable)]
     struct R {
@@ -767,9 +776,9 @@ pub fn summary(req: &mut Request) -> CargoResult<Response> {
     Ok(req.json(&R {
         num_downloads: num_downloads,
         num_crates: num_crates,
-        new_crates: to_crates(new_crates)?,
-        most_downloaded: to_crates(most_downloaded)?,
-        just_updated: to_crates(just_updated)?,
+        new_crates: encode_crates(new_crates)?,
+        most_downloaded: encode_crates(most_downloaded)?,
+        just_updated: encode_crates(just_updated)?,
         popular_keywords: popular_keywords,
         popular_categories: popular_categories,
     }))

src/version.rs

@@ -11,13 +11,13 @@ use url;
 
 use app::RequestApp;
 use db::RequestTransaction;
-use diesel::prelude::*;
-use diesel::pg::Pg;
 use dependency::{Dependency, EncodableDependency, Kind};
+use diesel::pg::Pg;
+use diesel::prelude::*;
 use download::{VersionDownload, EncodableVersionDownload};
 use git;
 use owner::{rights, Rights};
-use schema::versions;
+use schema::*;
 use upload;
 use user::RequestUser;
 use util::{RequestUtils, CargoResult, ChainError, internal, human};