Merge rust-lang#1287

1287: Finish MVC Split and Organize Middleware r=carols10cents

This PR largely finishes the reorganization of code into `models`, `views`, and `controllers` modules.  The following table summarizes the breakdown:

| File in src/ | Model | View | Controller |
| --- | --- | --- | --- |
| dependency.rs | X | X | |
| download.rs | X | X | |
| site_metadata.rs | | | X |
| token.rs | X | X | X |
| version/* | X | X | X|
| user/* | X | X | X |

While splitting `user/*` I created a new module for the CurrentUser middleware.  The rest of the commit series organizes and simplifies the remaining middleware logic and pulls the route building logic into `router.rs`.

The individual commits are straightforward, but the PR has grown rather large.  I can split it in half if that helps with review, but I hope its okay as-is.

Author: bors-voyager[bot]

src/app.rs

@@ -1,12 +1,9 @@
 //! Application-wide components in a struct accessible from each request
 
 use std::env;
-use std::error::Error;
 use std::path::PathBuf;
 use std::sync::{Arc, Mutex};
 
-use conduit::{Request, Response};
-use conduit_middleware::Middleware;
 use diesel::r2d2;
 use git2;
 use oauth2;
@@ -39,13 +36,6 @@ pub struct App {
     pub config: Config,
 }
 
-/// The `AppMiddleware` injects an `App` instance into the `Request` extensions
-// Can't derive Debug because `App` can't.
-#[allow(missing_debug_implementations)]
-pub struct AppMiddleware {
-    app: Arc<App>,
-}
-
 impl App {
     /// Creates a new `App` with a given `Config`
     ///
@@ -113,36 +103,3 @@ impl App {
         handle
     }
 }
-
-impl AppMiddleware {
-    pub fn new(app: Arc<App>) -> AppMiddleware {
-        AppMiddleware { app: app }
-    }
-}
-
-impl Middleware for AppMiddleware {
-    fn before(&self, req: &mut Request) -> Result<(), Box<Error + Send>> {
-        req.mut_extensions().insert(Arc::clone(&self.app));
-        Ok(())
-    }
-
-    fn after(
-        &self,
-        req: &mut Request,
-        res: Result<Response, Box<Error + Send>>,
-    ) -> Result<Response, Box<Error + Send>> {
-        req.mut_extensions().pop::<Arc<App>>().unwrap();
-        res
-    }
-}
-
-/// Adds an `app()` method to the `Request` type returning the global `App` instance
-pub trait RequestApp {
-    fn app(&self) -> &Arc<App>;
-}
-
-impl<T: Request + ?Sized> RequestApp for T {
-    fn app(&self) -> &Arc<App> {
-        self.extensions().find::<Arc<App>>().expect("Missing app")
-    }
-}

src/bin/delete-crate.rs

@@ -18,7 +18,6 @@ use std::io::prelude::*;
 use cargo_registry::models::Crate;
 use cargo_registry::schema::crates;
 
-#[allow(dead_code)]
 fn main() {
     let conn = cargo_registry::db::connect_now().unwrap();
     conn.transaction::<_, diesel::result::Error, _>(|| {

src/bin/delete-version.rs

@@ -18,7 +18,6 @@ use std::io::prelude::*;
 use cargo_registry::models::{Crate, Version};
 use cargo_registry::schema::versions;
 
-#[allow(dead_code)]
 fn main() {
     let conn = cargo_registry::db::connect_now().unwrap();
     conn.transaction::<_, diesel::result::Error, _>(|| {

src/bin/server.rs

@@ -12,7 +12,6 @@ use std::fs::{self, File};
 use std::sync::Arc;
 use std::sync::mpsc::channel;
 
-#[allow(dead_code)]
 fn main() {
     // Initialize logging
     env_logger::init();
@@ -46,7 +45,7 @@ fn main() {
     cfg.set_str("user.email", "[email protected]").unwrap();
 
     let app = cargo_registry::App::new(&config);
-    let app = cargo_registry::middleware(Arc::new(app));
+    let app = cargo_registry::build_handler(Arc::new(app));
 
     // On every server restart, ensure the categories available in the database match
     // the information in *src/categories.toml*.

src/bin/update-downloads.rs

@@ -13,7 +13,6 @@ use cargo_registry::schema::*;
 
 static LIMIT: i64 = 1000;
 
-#[allow(dead_code)] // dead in tests
 fn main() {
     let daemon = env::args().nth(1).as_ref().map(|s| &s[..]) == Some("daemon");
     let sleep = env::args().nth(2).map(|s| s.parse().unwrap());

src/controllers/krate/metadata.rs

@@ -4,8 +4,6 @@
 //! index or cached metadata which was extracted (client side) from the
 //! `Cargo.toml` file.
 
-use app::RequestApp;
-
 use controllers::prelude::*;
 use views::{EncodableCategory, EncodableCrate, EncodableDependency, EncodableKeyword,
             EncodableVersion};

src/controllers/krate/publish.rs

@@ -7,7 +7,6 @@ use std::sync::Arc;
 use hex::ToHex;
 use serde_json;
 
-use dependency;
 use git;
 use render;
 use util::{read_fill, read_le_u32};
@@ -16,6 +15,7 @@ use util::{internal, ChainError};
 use controllers::prelude::*;
 use views::{EncodableCrate, EncodableCrateUpload};
 use models::{Badge, Category, Keyword, NewCrate, NewVersion, Rights, User};
+use models::dependency;
 
 /// Handles the `PUT /crates/new` route.
 /// Used by `cargo publish` to publish a new crate or to publish a new version of an

src/controllers/mod.rs

@@ -1,15 +1,80 @@
-// TODO: Finish moving api endpoints to submodules here
-
 mod prelude {
     pub use diesel::prelude::*;
     pub use super::helpers::ok_true;
 
-    pub use app::RequestApp;
     pub use conduit::{Request, Response};
     pub use conduit_router::RequestParams;
+
     pub use db::RequestTransaction;
-    pub use user::RequestUser;
-    pub use util::{human, CargoResult, RequestUtils};
+    pub use util::{human, CargoResult};
+
+    pub use middleware::app::RequestApp;
+    pub use middleware::current_user::RequestUser;
+
+    use std::io;
+    use url;
+    use std::collections::HashMap;
+    use serde::Serialize;
+
+    pub trait RequestUtils {
+        fn redirect(&self, url: String) -> Response;
+
+        fn json<T: Serialize>(&self, t: &T) -> Response;
+        fn query(&self) -> HashMap<String, String>;
+        fn wants_json(&self) -> bool;
+        fn pagination(&self, default: usize, max: usize) -> CargoResult<(i64, i64)>;
+    }
+
+    impl<'a> RequestUtils for Request + 'a {
+        fn json<T: Serialize>(&self, t: &T) -> Response {
+            ::util::json_response(t)
+        }
+
+        fn query(&self) -> HashMap<String, String> {
+            url::form_urlencoded::parse(self.query_string().unwrap_or("").as_bytes())
+                .map(|(a, b)| (a.into_owned(), b.into_owned()))
+                .collect()
+        }
+
+        fn redirect(&self, url: String) -> Response {
+            let mut headers = HashMap::new();
+            headers.insert("Location".to_string(), vec![url.to_string()]);
+            Response {
+                status: (302, "Found"),
+                headers: headers,
+                body: Box::new(io::empty()),
+            }
+        }
+
+        fn wants_json(&self) -> bool {
+            self.headers()
+                .find("Accept")
+                .map(|accept| accept.iter().any(|s| s.contains("json")))
+                .unwrap_or(false)
+        }
+
+        fn pagination(&self, default: usize, max: usize) -> CargoResult<(i64, i64)> {
+            let query = self.query();
+            let page = query
+                .get("page")
+                .and_then(|s| s.parse::<usize>().ok())
+                .unwrap_or(1);
+            let limit = query
+                .get("per_page")
+                .and_then(|s| s.parse::<usize>().ok())
+                .unwrap_or(default);
+            if limit > max {
+                return Err(human(&format_args!(
+                    "cannot request more than {} items",
+                    max
+                )));
+            }
+            if page == 0 {
+                return Err(human("page indexing starts from 1, page 0 is invalid"));
+            }
+            Ok((((page - 1) * limit) as i64, limit as i64))
+        }
+    }
 }
 
 pub mod helpers;
@@ -18,3 +83,8 @@ pub mod category;
 pub mod crate_owner_invitation;
 pub mod keyword;
 pub mod krate;
+pub mod site_metadata;
+pub mod team;
+pub mod token;
+pub mod version;
+pub mod user;

src/site_metadata.rs renamed to src/controllers/site_metadata.rs

@@ -1,6 +1,4 @@
-use conduit::{Request, Response};
-use util::RequestUtils;
-use util::errors::CargoResult;
+use super::prelude::*;
 
 /// Returns the JSON representation of the current deployed commit sha.
 ///

src/controllers/team.rs

@@ -0,0 +1,22 @@
+use controllers::prelude::*;
+
+use models::Team;
+use schema::teams;
+use views::EncodableTeam;
+
+/// Handles the `GET /teams/:team_id` route.
+pub fn show_team(req: &mut Request) -> CargoResult<Response> {
+    use self::teams::dsl::{login, teams};
+
+    let name = &req.params()["team_id"];
+    let conn = req.db_conn()?;
+    let team = teams.filter(login.eq(name)).first::<Team>(&*conn)?;
+
+    #[derive(Serialize)]
+    struct R {
+        team: EncodableTeam,
+    }
+    Ok(req.json(&R {
+        team: team.encodable(),
+    }))
+}

src/controllers/token.rs

@@ -0,0 +1,100 @@
+use super::prelude::*;
+
+use diesel;
+use serde_json as json;
+use middleware::current_user::AuthenticationSource;
+use util::{bad_request, read_fill, ChainError};
+
+use models::ApiToken;
+use schema::api_tokens;
+use views::EncodableApiTokenWithToken;
+
+/// Handles the `GET /me/tokens` route.
+pub fn list(req: &mut Request) -> CargoResult<Response> {
+    let tokens = ApiToken::belonging_to(req.user()?)
+        .order(api_tokens::created_at.desc())
+        .load(&*req.db_conn()?)?;
+    #[derive(Serialize)]
+    struct R {
+        api_tokens: Vec<ApiToken>,
+    }
+    Ok(req.json(&R { api_tokens: tokens }))
+}
+
+/// Handles the `PUT /me/tokens` route.
+pub fn new(req: &mut Request) -> CargoResult<Response> {
+    /// The incoming serialization format for the `ApiToken` model.
+    #[derive(Deserialize, Serialize)]
+    struct NewApiToken {
+        name: String,
+    }
+
+    /// The incoming serialization format for the `ApiToken` model.
+    #[derive(Deserialize, Serialize)]
+    struct NewApiTokenRequest {
+        api_token: NewApiToken,
+    }
+
+    if req.authentication_source()? != AuthenticationSource::SessionCookie {
+        return Err(bad_request(
+            "cannot use an API token to create a new API token",
+        ));
+    }
+
+    let max_size = 2000;
+    let length = req.content_length()
+        .chain_error(|| bad_request("missing header: Content-Length"))?;
+
+    if length > max_size {
+        return Err(bad_request(&format!("max content length is: {}", max_size)));
+    }
+
+    let mut json = vec![0; length as usize];
+    read_fill(req.body(), &mut json)?;
+
+    let json = String::from_utf8(json).map_err(|_| bad_request(&"json body was not valid utf-8"))?;
+
+    let new: NewApiTokenRequest = json::from_str(&json)
+        .map_err(|e| bad_request(&format!("invalid new token request: {:?}", e)))?;
+
+    let name = &new.api_token.name;
+    if name.len() < 1 {
+        return Err(bad_request("name must have a value"));
+    }
+
+    let user = req.user()?;
+
+    let max_token_per_user = 500;
+    let count = ApiToken::belonging_to(user)
+        .count()
+        .get_result::<i64>(&*req.db_conn()?)?;
+    if count >= max_token_per_user {
+        return Err(bad_request(&format!(
+            "maximum tokens per user is: {}",
+            max_token_per_user
+        )));
+    }
+
+    let api_token = ApiToken::insert(&*req.db_conn()?, user.id, name)?;
+
+    #[derive(Serialize)]
+    struct R {
+        api_token: EncodableApiTokenWithToken,
+    }
+    Ok(req.json(&R {
+        api_token: api_token.encodable_with_token(),
+    }))
+}
+
+/// Handles the `DELETE /me/tokens/:id` route.
+pub fn revoke(req: &mut Request) -> CargoResult<Response> {
+    let id = req.params()["id"]
+        .parse::<i32>()
+        .map_err(|e| bad_request(&format!("invalid token id: {:?}", e)))?;
+
+    diesel::delete(ApiToken::belonging_to(req.user()?).find(id)).execute(&*req.db_conn()?)?;
+
+    #[derive(Serialize)]
+    struct R {}
+    Ok(req.json(&R {}))
+}