Merge pull request #882 from jtgeibel/prefer-queryresult-over-cargoresult

Use QueryResult in place of CargoResult where possible and remove some unused code

Author: carols10cents

src/badge.rs

@@ -1,6 +1,5 @@
 use krate::Crate;
 use schema::badges;
-use util::CargoResult;
 
 use diesel::pg::{Pg, PgConnection};
 use diesel::prelude::*;
@@ -79,7 +78,7 @@ impl Badge {
         conn: &PgConnection,
         krate: &Crate,
         badges: Option<&'a HashMap<String, HashMap<String, String>>>,
-    ) -> CargoResult<Vec<&'a str>> {
+    ) -> QueryResult<Vec<&'a str>> {
         use diesel::{insert, delete};
 
         #[derive(Insertable)]

src/category.rs

@@ -4,13 +4,11 @@ use conduit::{Request, Response};
 use conduit_router::RequestParams;
 use diesel::*;
 use diesel::pg::PgConnection;
-use pg::GenericConnection;
 use pg::rows::Row;
 
 use db::RequestTransaction;
 use schema::*;
-use util::errors::NotFound;
-use util::{RequestUtils, CargoResult, ChainError};
+use util::{RequestUtils, CargoResult};
 use {Model, Crate};
 
 #[derive(Clone, Identifiable, Queryable, Debug)]
@@ -56,17 +54,6 @@ pub struct EncodableCategoryWithSubcategories {
 }
 
 impl Category {
-    pub fn find_by_category(conn: &GenericConnection, name: &str) -> CargoResult<Category> {
-        let stmt = conn.prepare(
-            "SELECT * FROM categories \
-             WHERE category = $1",
-        )?;
-        let rows = stmt.query(&[&name])?;
-        rows.iter().next().chain_error(|| NotFound).map(|row| {
-            Model::from_row(&row)
-        })
-    }
-
     pub fn encodable(self) -> EncodableCategory {
         let Category {
             crates_cnt,
@@ -159,39 +146,6 @@ impl Category {
         ))).load(conn)
     }
 
-    pub fn toplevel_old(
-        conn: &GenericConnection,
-        sort: &str,
-        limit: i64,
-        offset: i64,
-    ) -> CargoResult<Vec<Category>> {
-
-        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
-        let stmt = conn.prepare(&format!(
-            "SELECT c.id, c.category, c.slug, c.description, c.created_at,
-                sum(c2.crates_cnt)::int as crates_cnt
-             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 $1 OFFSET $2",
-            sort_sql
-        ))?;
-
-        let categories: Vec<_> = stmt.query(&[&limit, &offset])?
-            .iter()
-            .map(|row| Model::from_row(&row))
-            .collect();
-
-        Ok(categories)
-    }
-
     pub fn subcategories(&self, conn: &PgConnection) -> QueryResult<Vec<Category>> {
         use diesel::expression::dsl::*;
         use diesel::types::Text;
@@ -221,7 +175,7 @@ pub struct NewCategory<'a> {
 
 impl<'a> NewCategory<'a> {
     /// Inserts the category into the database, or updates an existing one.
-    pub fn create_or_update(&self, conn: &PgConnection) -> CargoResult<Category> {
+    pub fn create_or_update(&self, conn: &PgConnection) -> QueryResult<Category> {
         use diesel::insert;
         use diesel::pg::upsert::*;
 

src/dependency.rs

@@ -1,7 +1,6 @@
 use diesel::prelude::*;
 use diesel::pg::{Pg, PgConnection};
 use diesel::types::{Integer, Text};
-use pg::GenericConnection;
 use pg::rows::Row;
 use semver;
 
@@ -72,42 +71,6 @@ pub struct NewDependency<'a> {
 }
 
 impl Dependency {
-    // FIXME: Encapsulate this in a `NewDependency` struct
-    #[cfg_attr(feature = "clippy", allow(too_many_arguments))]
-    pub fn insert(
-        conn: &GenericConnection,
-        version_id: i32,
-        crate_id: i32,
-        req: &semver::VersionReq,
-        kind: Kind,
-        optional: bool,
-        default_features: bool,
-        features: &[String],
-        target: &Option<String>,
-    ) -> CargoResult<Dependency> {
-        let req = req.to_string();
-        let stmt = conn.prepare(
-            "INSERT INTO dependencies
-                                      (version_id, crate_id, req, optional,
-                                       default_features, features, target, kind)
-                                      VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
-                                      RETURNING *",
-        )?;
-        let rows = stmt.query(
-            &[
-                &version_id,
-                &crate_id,
-                &req,
-                &optional,
-                &default_features,
-                &features,
-                target,
-                &(kind as i32),
-            ],
-        )?;
-        Ok(Model::from_row(&rows.iter().next().unwrap()))
-    }
-
     // `downloads` need only be specified when generating a reverse dependency
     pub fn encodable(self, crate_name: &str, downloads: Option<i32>) -> EncodableDependency {
         EncodableDependency {

src/krate.rs

@@ -241,7 +241,7 @@ impl<'a> NewCrate<'a> {
         }
     }
 
-    fn save_new_crate(&self, conn: &PgConnection, user_id: i32) -> CargoResult<Option<Crate>> {
+    fn save_new_crate(&self, conn: &PgConnection, user_id: i32) -> QueryResult<Option<Crate>> {
         use schema::crates::dsl::*;
         use diesel::insert;
 
@@ -554,19 +554,6 @@ impl Crate {
         Ok(Version::max(vs))
     }
 
-    pub fn max_version_old(&self, conn: &GenericConnection) -> CargoResult<semver::Version> {
-        let stmt = conn.prepare(
-            "SELECT num FROM versions WHERE crate_id = $1
-                                 AND yanked = 'f'",
-        )?;
-        let rows = stmt.query(&[&self.id])?;
-        Ok(Version::max(
-            rows.iter().map(|r| r.get::<_, String>("num")).map(|s| {
-                semver::Version::parse(&s).unwrap()
-            }),
-        ))
-    }
-
     pub fn versions(&self, conn: &GenericConnection) -> CargoResult<Vec<Version>> {
         let stmt = conn.prepare(
             "SELECT * FROM versions \
@@ -600,6 +587,7 @@ impl Crate {
         Ok(users.chain(teams).collect())
     }
 
+    // TODO: Update bin/transfer_crates to use owners() then get rid of this
     pub fn owners_old(&self, conn: &GenericConnection) -> CargoResult<Vec<Owner>> {
         let stmt = conn.prepare(
             "SELECT * FROM users
@@ -687,44 +675,6 @@ impl Crate {
         Ok(())
     }
 
-    pub fn add_version(
-        &mut self,
-        conn: &GenericConnection,
-        ver: &semver::Version,
-        features: &HashMap<String, Vec<String>>,
-        authors: &[String],
-    ) -> CargoResult<Version> {
-        if Version::find_by_num(conn, self.id, ver)?.is_some() {
-            return Err(human(
-                &format_args!("crate version `{}` is already uploaded", ver),
-            ));
-        }
-        Version::insert(conn, self.id, ver, features, authors)
-    }
-
-    pub fn keywords(&self, conn: &GenericConnection) -> CargoResult<Vec<Keyword>> {
-        let stmt = conn.prepare(
-            "SELECT keywords.* FROM keywords
-                                      LEFT JOIN crates_keywords
-                                      ON keywords.id = crates_keywords.keyword_id
-                                      WHERE crates_keywords.crate_id = $1",
-        )?;
-        let rows = stmt.query(&[&self.id])?;
-        Ok(rows.iter().map(|r| Model::from_row(&r)).collect())
-    }
-
-    pub fn categories(&self, conn: &GenericConnection) -> CargoResult<Vec<Category>> {
-        let stmt = conn.prepare(
-            "SELECT categories.* FROM categories \
-             LEFT JOIN crates_categories \
-             ON categories.id = \
-             crates_categories.category_id \
-             WHERE crates_categories.crate_id = $1",
-        )?;
-        let rows = stmt.query(&[&self.id])?;
-        Ok(rows.iter().map(|r| Model::from_row(&r)).collect())
-    }
-
     pub fn badges(&self, conn: &PgConnection) -> QueryResult<Vec<Badge>> {
         badges::table.filter(badges::crate_id.eq(self.id)).load(
             conn,
@@ -737,7 +687,7 @@ impl Crate {
         conn: &PgConnection,
         offset: i64,
         limit: i64,
-    ) -> CargoResult<(Vec<ReverseDependency>, i64)> {
+    ) -> QueryResult<(Vec<ReverseDependency>, i64)> {
         use diesel::expression::dsl::sql;
         use diesel::types::{Integer, Text, BigInt};
 

src/token.rs

@@ -45,7 +45,7 @@ pub struct EncodableApiTokenWithToken {
 
 impl ApiToken {
     /// Generates a new named API token for a user
-    pub fn insert(conn: &PgConnection, user_id: i32, name: &str) -> CargoResult<ApiToken> {
+    pub fn insert(conn: &PgConnection, user_id: i32, name: &str) -> QueryResult<ApiToken> {
         #[table_name = "api_tokens"]
         #[derive(Insertable, AsChangeset)]
         struct NewApiToken<'a> {
@@ -62,14 +62,14 @@ impl ApiToken {
     }
 
     /// Deletes the provided API token if it belongs to the provided user
-    pub fn delete(conn: &PgConnection, user_id: i32, id: i32) -> CargoResult<()> {
+    pub fn delete(conn: &PgConnection, user_id: i32, id: i32) -> QueryResult<()> {
         diesel::delete(api_tokens::table.find(id).filter(
             api_tokens::user_id.eq(user_id),
         )).execute(conn)?;
         Ok(())
     }
 
-    pub fn find_for_user(conn: &PgConnection, user_id: i32) -> CargoResult<Vec<ApiToken>> {
+    pub fn find_for_user(conn: &PgConnection, user_id: i32) -> QueryResult<Vec<ApiToken>> {
         api_tokens::table
             .filter(api_tokens::user_id.eq(user_id))
             .order(api_tokens::created_at.desc())

src/uploaders.rs

@@ -153,7 +153,7 @@ impl Uploader {
         }
     }
 
-    pub fn delete(&self, app: Arc<App>, path: &str) -> CargoResult<()> {
+    fn delete(&self, app: Arc<App>, path: &str) -> CargoResult<()> {
         match *self {
             Uploader::S3 { ref bucket, .. } => {
                 let mut handle = app.handle();

src/user/mod.rs

@@ -67,7 +67,7 @@ impl<'a> NewUser<'a> {
     }
 
     /// Inserts the user into the database, or updates an existing one.
-    pub fn create_or_update(&self, conn: &PgConnection) -> CargoResult<User> {
+    pub fn create_or_update(&self, conn: &PgConnection) -> QueryResult<User> {
         use diesel::insert;
         use diesel::expression::dsl::sql;
         use diesel::types::Integer;

src/version.rs

@@ -6,6 +6,7 @@ use diesel;
 use diesel::pg::{Pg, PgConnection};
 use diesel::prelude::*;
 use pg::GenericConnection;
+use pg::Result as PgResult;
 use pg::rows::Row;
 use semver;
 use serde_json;
@@ -76,7 +77,7 @@ impl Version {
         conn: &GenericConnection,
         crate_id: i32,
         num: &semver::Version,
-    ) -> CargoResult<Option<Version>> {
+    ) -> PgResult<Option<Version>> {
         let num = num.to_string();
         let stmt = conn.prepare(
             "SELECT * FROM versions \
@@ -111,10 +112,6 @@ impl Version {
         Ok(ret)
     }
 
-    pub fn valid(version: &str) -> bool {
-        semver::Version::parse(version).is_ok()
-    }
-
     pub fn encodable(self, crate_name: &str) -> EncodableVersion {
         let Version {
             id,
@@ -156,7 +153,7 @@ impl Version {
             .load(conn)
     }
 
-    pub fn add_author(&self, conn: &GenericConnection, name: &str) -> CargoResult<()> {
+    pub fn add_author(&self, conn: &GenericConnection, name: &str) -> PgResult<()> {
         conn.execute(
             "INSERT INTO version_authors (version_id, name)
                            VALUES ($1, $2)",
@@ -165,14 +162,6 @@ impl Version {
         Ok(())
     }
 
-    pub fn yank(&self, conn: &GenericConnection, yanked: bool) -> CargoResult<()> {
-        conn.execute(
-            "UPDATE versions SET yanked = $1 WHERE id = $2",
-            &[&yanked, &self.id],
-        )?;
-        Ok(())
-    }
-
     pub fn max<T>(versions: T) -> semver::Version
     where
         T: IntoIterator<Item = semver::Version>,