Move download endpoint from krate::cargo to version::cargo

This endpoint is for a specific version, not the crate in general.

Author: jtgeibel

src/krate/cache.rs

@@ -1,5 +1,5 @@
 //! Endpoints that provide cached data about the published crates
-//! 
+//!
 //! Some of these endpoints include download counts, but other than that this
 //! information could be reconstructed from the index and crate metadata.
 

src/krate/cargo.rs

@@ -15,7 +15,6 @@ use serde_json;
 use app::RequestApp;
 use db::RequestTransaction;
 use dependency;
-use download::VersionDownload;
 use git;
 use owner::{rights, EncodableOwner, Owner, OwnerKind, Rights};
 use pagination::Paginate;
@@ -26,7 +25,7 @@ use user::RequestUser;
 use util::{read_fill, read_le_u32};
 use util::{human, internal, CargoResult, ChainError, RequestUtils};
 use version::NewVersion;
-use {Badge, Category, Keyword, Replica, User, Version};
+use {Badge, Category, Keyword, User, Version};
 
 use super::{canon_crate_name, Crate, EncodableCrate, NewCrate, ALL_COLUMNS};
 
@@ -445,53 +444,6 @@ fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)>
     Ok((new, user.clone()))
 }
 
-/// Handles the `GET /crates/:crate_id/:version/download` route.
-/// This returns a URL to the location where the crate is stored.
-pub fn download(req: &mut Request) -> CargoResult<Response> {
-    let crate_name = &req.params()["crate_id"];
-    let version = &req.params()["version"];
-
-    // If we are a mirror, ignore failure to update download counts.
-    // API-only mirrors won't have any crates in their database, and
-    // incrementing the download count will look up the crate in the
-    // database. Mirrors just want to pass along a redirect URL.
-    if req.app().config.mirror == Replica::ReadOnlyMirror {
-        let _ = increment_download_counts(req, crate_name, version);
-    } else {
-        increment_download_counts(req, crate_name, version)?;
-    }
-
-    let redirect_url = req.app()
-        .config
-        .uploader
-        .crate_location(crate_name, version)
-        .ok_or_else(|| human("crate files not found"))?;
-
-    if req.wants_json() {
-        #[derive(Serialize)]
-        struct R {
-            url: String,
-        }
-        Ok(req.json(&R { url: redirect_url }))
-    } else {
-        Ok(req.redirect(redirect_url))
-    }
-}
-
-fn increment_download_counts(req: &Request, crate_name: &str, version: &str) -> CargoResult<()> {
-    use self::versions::dsl::*;
-
-    let conn = req.db_conn()?;
-    let version_id = versions
-        .select(id)
-        .filter(crate_id.eq_any(Crate::by_name(crate_name).select(crates::id)))
-        .filter(num.eq(version))
-        .first(&*conn)?;
-
-    VersionDownload::create_or_increment(version_id, &conn)?;
-    Ok(())
-}
-
 /// Handles the `GET /crates/:crate_id/owners` route.
 pub fn owners(req: &mut Request) -> CargoResult<Response> {
     let crate_name = &req.params()["crate_id"];

src/lib.rs

@@ -142,13 +142,14 @@ pub fn middleware(app: Arc<App>) -> MiddlewareBuilder {
 
     // Routes used by `cargo`
     api_router.put("/crates/new", C(krate::cargo::new));
-    api_router.get(
-        "/crates/:crate_id/:version/download",
-        C(krate::cargo::download),
-    );
     api_router.get("/crates/:crate_id/owners", C(krate::cargo::owners));
     api_router.put("/crates/:crate_id/owners", C(krate::cargo::add_owners));
     api_router.delete("/crates/:crate_id/owners", C(krate::cargo::remove_owners));
+
+    api_router.get(
+        "/crates/:crate_id/:version/download",
+        C(version::cargo::download),
+    );
     api_router.delete("/crates/:crate_id/:version/yank", C(version::cargo::yank));
     api_router.put(
         "/crates/:crate_id/:version/unyank",

src/version/cargo.rs

@@ -16,7 +16,58 @@ use user::RequestUser;
 use util::errors::CargoError;
 use util::{human, CargoResult, RequestUtils};
 
-use super::version_and_crate;
+use super::{version_and_crate, Crate};
+
+/// Handles the `GET /crates/:crate_id/:version/download` route.
+/// This returns a URL to the location where the crate is stored.
+pub fn download(req: &mut Request) -> CargoResult<Response> {
+    use Replica;
+    use conduit_router::RequestParams;
+
+    let crate_name = &req.params()["crate_id"];
+    let version = &req.params()["version"];
+
+    // If we are a mirror, ignore failure to update download counts.
+    // API-only mirrors won't have any crates in their database, and
+    // incrementing the download count will look up the crate in the
+    // database. Mirrors just want to pass along a redirect URL.
+    if req.app().config.mirror == Replica::ReadOnlyMirror {
+        let _ = increment_download_counts(req, crate_name, version);
+    } else {
+        increment_download_counts(req, crate_name, version)?;
+    }
+
+    let redirect_url = req.app()
+        .config
+        .uploader
+        .crate_location(crate_name, version)
+        .ok_or_else(|| human("crate files not found"))?;
+
+    if req.wants_json() {
+        #[derive(Serialize)]
+        struct R {
+            url: String,
+        }
+        Ok(req.json(&R { url: redirect_url }))
+    } else {
+        Ok(req.redirect(redirect_url))
+    }
+}
+
+fn increment_download_counts(req: &Request, crate_name: &str, version: &str) -> CargoResult<()> {
+    use download::VersionDownload;
+    use self::versions::dsl::*;
+
+    let conn = req.db_conn()?;
+    let version_id = versions
+        .select(id)
+        .filter(crate_id.eq_any(Crate::by_name(crate_name).select(crates::id)))
+        .filter(num.eq(version))
+        .first(&*conn)?;
+
+    VersionDownload::create_or_increment(version_id, &conn)?;
+    Ok(())
+}
 
 /// Handles the `DELETE /crates/:crate_id/:version/yank` route.
 /// This does not delete a crate version, it makes the crate