Move several version routes to version::deprecated::*

Author: jtgeibel

src/lib.rs

@@ -153,12 +153,12 @@ pub fn middleware(app: Arc<App>) -> MiddlewareBuilder {
     );
 
     // Routes that appear to be unused
-    api_router.get("/versions", C(version::index));
-    api_router.get("/versions/:version_id", C(version::show));
+    api_router.get("/versions", C(version::deprecated::index));
+    api_router.get("/versions/:version_id", C(version::deprecated::show));
 
     // Routes used by the frontend
     api_router.get("/crates/:crate_id", C(krate::metadata::show));
-    api_router.get("/crates/:crate_id/:version", C(version::show));
+    api_router.get("/crates/:crate_id/:version", C(version::deprecated::show));
     api_router.get(
         "/crates/:crate_id/:version/readme",
         C(krate::metadata::readme),

src/version/deprecated.rs

@@ -0,0 +1,80 @@
+//! Deprecated api endpoints
+//!
+//! There are no known uses of these endpoints.  There is currently no plan for
+//! removing these endpoints.  At a minimum, logs should be reviewed over a
+//! period of time to ensure there are no external users of an endpoint before
+//! it is removed.
+
+use conduit_router::RequestParams;
+use conduit::{Request, Response};
+use diesel::prelude::*;
+use url;
+
+use db::RequestTransaction;
+use schema::*;
+use util::{CargoResult, RequestUtils};
+
+use super::{version_and_crate, EncodableVersion, Version};
+
+/// Handles the `GET /versions` route.
+pub fn index(req: &mut Request) -> CargoResult<Response> {
+    use diesel::expression::dsl::any;
+    let conn = req.db_conn()?;
+
+    // Extract all ids requested.
+    let query = url::form_urlencoded::parse(req.query_string().unwrap_or("").as_bytes());
+    let ids = query
+        .filter_map(|(ref a, ref b)| if *a == "ids[]" {
+            b.parse().ok()
+        } else {
+            None
+        })
+        .collect::<Vec<i32>>();
+
+    let versions = versions::table
+        .inner_join(crates::table)
+        .select((versions::all_columns, crates::name))
+        .filter(versions::id.eq(any(ids)))
+        .load::<(Version, String)>(&*conn)?
+        .into_iter()
+        .map(|(version, crate_name)| version.encodable(&crate_name))
+        .collect();
+
+    #[derive(Serialize)]
+    struct R {
+        versions: Vec<EncodableVersion>,
+    }
+    Ok(req.json(&R { versions: 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 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, ::krate::ALL_COLUMNS))
+                .first(&*conn)?
+        }
+    };
+
+    #[derive(Serialize)]
+    struct R {
+        version: EncodableVersion,
+    }
+    Ok(req.json(&R {
+        version: version.encodable(&krate.name),
+    }))
+}

src/version/mod.rs

@@ -8,7 +8,6 @@ use diesel::pg::Pg;
 use diesel::prelude::*;
 use semver;
 use serde_json;
-use url;
 
 use Crate;
 use app::RequestApp;
@@ -22,6 +21,7 @@ use util::errors::CargoError;
 use util::{human, CargoResult, RequestUtils};
 use license_exprs;
 
+pub mod deprecated;
 pub mod downloads;
 
 // Queryable has a custom implementation below
@@ -259,63 +259,6 @@ impl Queryable<versions::SqlType, Pg> for Version {
     }
 }
 
-/// Handles the `GET /versions` route.
-// FIXME: where/how is this used?
-pub fn index(req: &mut Request) -> CargoResult<Response> {
-    use diesel::expression::dsl::any;
-    let conn = req.db_conn()?;
-
-    // Extract all ids requested.
-    let query = url::form_urlencoded::parse(req.query_string().unwrap_or("").as_bytes());
-    let ids = query
-        .filter_map(|(ref a, ref b)| if *a == "ids[]" {
-            b.parse().ok()
-        } else {
-            None
-        })
-        .collect::<Vec<i32>>();
-
-    let versions = versions::table
-        .inner_join(crates::table)
-        .select((versions::all_columns, crates::name))
-        .filter(versions::id.eq(any(ids)))
-        .load::<(Version, String)>(&*conn)?
-        .into_iter()
-        .map(|(version, crate_name)| version.encodable(&crate_name))
-        .collect();
-
-    #[derive(Serialize)]
-    struct R {
-        versions: Vec<EncodableVersion>,
-    }
-    Ok(req.json(&R { versions: versions }))
-}
-
-/// Handles the `GET /versions/:version_id` route.
-pub fn show(req: &mut 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, ::krate::ALL_COLUMNS))
-                .first(&*conn)?
-        }
-    };
-
-    #[derive(Serialize)]
-    struct R {
-        version: EncodableVersion,
-    }
-    Ok(req.json(&R {
-        version: version.encodable(&krate.name),
-    }))
-}
-
 fn version_and_crate(req: &mut Request) -> CargoResult<(Version, Crate)> {
     let crate_name = &req.params()["crate_id"];
     let semver = &req.params()["version"];