diff --git a/app/controllers/dashboard.js b/app/controllers/dashboard.js index 3708b4c7866..0396f21c613 100644 --- a/app/controllers/dashboard.js +++ b/app/controllers/dashboard.js @@ -38,9 +38,6 @@ export default Ember.Controller.extend({ var page = (this.get('myFeed').length / 10) + 1; ajax(`/me/updates?page=${page}`).then((data) => { - data.crates.forEach(crate => - this.store.push(this.store.normalize('crate', crate))); - var versions = data.versions.map(version => this.store.push(this.store.normalize('version', version))); diff --git a/app/models/version.js b/app/models/version.js index 0876d3cdf89..5f6a7343ef8 100644 --- a/app/models/version.js +++ b/app/models/version.js @@ -1,4 +1,5 @@ import DS from 'ember-data'; +import Ember from 'ember'; export default DS.Model.extend({ num: DS.attr('string'), @@ -15,6 +16,10 @@ export default DS.Model.extend({ dependencies: DS.hasMany('dependency', { async: true }), version_downloads: DS.hasMany('version-download', { async: true }), + crateName: Ember.computed('crate', function() { + return this.belongsTo('crate').id(); + }), + getDownloadUrl() { return this.store.adapterFor('version').getDownloadUrl(this.get('dl_path')); }, diff --git a/app/templates/dashboard.hbs b/app/templates/dashboard.hbs index f0b074e6f3d..0f0ed93f2ee 100644 --- a/app/templates/dashboard.hbs +++ b/app/templates/dashboard.hbs @@ -49,10 +49,12 @@ {{#each myFeed as |version|}}
- {{link-to version.crate.name 'crate.version' version.num}} + {{#link-to 'crate.version' version.crateName version.num}} + {{ version.crateName }} {{ version.num }} + {{/link-to}} - {{from-now version.created_at}} + {{moment-from-now version.created_at}}
diff --git a/src/krate.rs b/src/krate.rs index 20e2c959e50..7f973c790a0 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -4,9 +4,11 @@ use std::collections::HashMap; use conduit::{Request, Response}; use conduit_router::RequestParams; -use diesel::pg::PgConnection; +use diesel::associations::Identifiable; use diesel::pg::upsert::*; +use diesel::pg::{Pg, PgConnection}; use diesel::prelude::*; +use diesel; use diesel_full_text_search::*; use license_exprs; use pg::GenericConnection; @@ -35,7 +37,8 @@ use util::{RequestUtils, CargoResult, internal, ChainError, human}; use version::EncodableVersion; use {Model, User, Keyword, Version, Category, Badge, Replica}; -#[derive(Clone, Queryable, Identifiable, AsChangeset)] +#[derive(Clone, Queryable, Identifiable, AsChangeset, Associations)] +#[has_many(versions)] pub struct Crate { pub id: i32, pub name: String, @@ -64,6 +67,8 @@ pub const ALL_COLUMNS: AllColumns = (crates::id, crates::name, crates::readme, crates::license, crates::repository, crates::max_upload_size); +type CrateQuery<'a> = crates::BoxedQuery<'a, Pg, ::SqlType>; + #[derive(RustcEncodable, RustcDecodable)] pub struct EncodableCrate { pub id: String, @@ -224,6 +229,15 @@ impl<'a> NewCrate<'a> { } impl Crate { + pub fn by_name(name: &str) -> CrateQuery { + crates::table + .select(ALL_COLUMNS) + .filter( + canon_crate_name(crates::name).eq( + canon_crate_name(name)) + ).into_boxed() + } + pub fn find_by_name(conn: &GenericConnection, name: &str) -> CargoResult { let stmt = conn.prepare("SELECT * FROM crates \ @@ -1093,17 +1107,35 @@ fn user_and_crate(req: &mut Request) -> CargoResult<(User, Crate)> { Ok((user.clone(), krate)) } +#[derive(Insertable, Queryable, Identifiable, Associations)] +#[belongs_to(User)] +#[primary_key(user_id, crate_id)] +#[table_name="follows"] +pub struct Follow { + user_id: i32, + crate_id: i32, +} + +fn follow_target(req: &mut Request) -> CargoResult { + let user = req.user()?; + let conn = req.db_conn()?; + let crate_name = &req.params()["crate_id"]; + let crate_id = Crate::by_name(crate_name) + .select(crates::id) + .first(conn)?; + Ok(Follow { + user_id: user.id, + crate_id: crate_id, + }) +} + /// Handles the `PUT /crates/:crate_id/follow` route. pub fn follow(req: &mut Request) -> CargoResult { - let (user, krate) = user_and_crate(req)?; - let tx = req.tx()?; - let stmt = tx.prepare("SELECT 1 FROM follows - WHERE user_id = $1 AND crate_id = $2")?; - let rows = stmt.query(&[&user.id, &krate.id])?; - if !rows.iter().next().is_some() { - tx.execute("INSERT INTO follows (user_id, crate_id) - VALUES ($1, $2)", &[&user.id, &krate.id])?; - } + let follow = follow_target(req)?; + let conn = req.db_conn()?; + diesel::insert(&follow.on_conflict_do_nothing()) + .into(follows::table) + .execute(conn)?; #[derive(RustcEncodable)] struct R { ok: bool } Ok(req.json(&R { ok: true })) @@ -1111,11 +1143,9 @@ pub fn follow(req: &mut Request) -> CargoResult { /// Handles the `DELETE /crates/:crate_id/follow` route. pub fn unfollow(req: &mut Request) -> CargoResult { - let (user, krate) = user_and_crate(req)?; - let tx = req.tx()?; - tx.execute("DELETE FROM follows - WHERE user_id = $1 AND crate_id = $2", - &[&user.id, &krate.id])?; + let follow = follow_target(req)?; + let conn = req.db_conn()?; + diesel::delete(&follow).execute(conn)?; #[derive(RustcEncodable)] struct R { ok: bool } Ok(req.json(&R { ok: true })) @@ -1123,14 +1153,15 @@ pub fn unfollow(req: &mut Request) -> CargoResult { /// Handles the `GET /crates/:crate_id/following` route. pub fn following(req: &mut Request) -> CargoResult { - let (user, krate) = user_and_crate(req)?; - let tx = req.tx()?; - let stmt = tx.prepare("SELECT 1 FROM follows - WHERE user_id = $1 AND crate_id = $2")?; - let rows = stmt.query(&[&user.id, &krate.id])?; + use diesel::expression::dsl::exists; + + let follow = follow_target(req)?; + let conn = req.db_conn()?; + let following = diesel::select(exists(follows::table.find(follow.id()))) + .get_result(conn)?; #[derive(RustcEncodable)] struct R { following: bool } - Ok(req.json(&R { following: rows.iter().next().is_some() })) + Ok(req.json(&R { following: following })) } /// Handles the `GET /crates/:crate_id/versions` route. diff --git a/src/tests/all.rs b/src/tests/all.rs index 7e2375ab64e..06dedadf8f9 100755 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -1,12 +1,13 @@ #![deny(warnings)] +#[macro_use] extern crate diesel; +#[macro_use] extern crate diesel_codegen; extern crate bufstream; extern crate cargo_registry; extern crate conduit; extern crate conduit_middleware; extern crate conduit_test; extern crate curl; -extern crate diesel; extern crate dotenv; extern crate git2; extern crate postgres; diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 83a8cb6088f..57c9e9cf719 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -769,63 +769,53 @@ fn dependencies() { #[test] fn following() { - // #[derive(RustcDecodable)] struct F { following: bool } - // #[derive(RustcDecodable)] struct O { ok: bool } + #[derive(RustcDecodable)] struct F { following: bool } + #[derive(RustcDecodable)] struct O { ok: bool } let (_b, app, middle) = ::app(); let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates/foo_following/following"); let user; - let krate; { let conn = app.diesel_database.get().unwrap(); user = ::new_user("foo").create_or_update(&conn).unwrap(); ::sign_in_as(&mut req, &user); - krate = ::new_crate("foo_following").create_or_update(&conn, None, user.id).unwrap(); - - // FIXME: Go back to hitting the actual endpoint once it's using Diesel - conn - .execute(&format!("INSERT INTO follows (user_id, crate_id) VALUES ({}, {})", - user.id, krate.id)) - .unwrap(); + ::new_crate("foo_following").create_or_update(&conn, None, user.id).unwrap(); } - // let mut response = ok_resp!(middle.call(&mut req)); - // assert!(!::json::(&mut response).following); + let mut response = ok_resp!(middle.call(&mut req)); + assert!(!::json::(&mut response).following); - // req.with_path("/api/v1/crates/foo_following/follow") - // .with_method(Method::Put); - // let mut response = ok_resp!(middle.call(&mut req)); - // assert!(::json::(&mut response).ok); - // let mut response = ok_resp!(middle.call(&mut req)); - // assert!(::json::(&mut response).ok); + req.with_path("/api/v1/crates/foo_following/follow") + .with_method(Method::Put); + let mut response = ok_resp!(middle.call(&mut req)); + assert!(::json::(&mut response).ok); + let mut response = ok_resp!(middle.call(&mut req)); + assert!(::json::(&mut response).ok); - // req.with_path("/api/v1/crates/foo_following/following") - // .with_method(Method::Get); - // let mut response = ok_resp!(middle.call(&mut req)); - // assert!(::json::(&mut response).following); + req.with_path("/api/v1/crates/foo_following/following") + .with_method(Method::Get); + let mut response = ok_resp!(middle.call(&mut req)); + assert!(::json::(&mut response).following); req.with_path("/api/v1/crates") - .with_query("following=1"); + .with_method(Method::Get) + .with_query("following=1"); let mut response = ok_resp!(middle.call(&mut req)); let l = ::json::(&mut response); assert_eq!(l.crates.len(), 1); - // FIXME: Go back to hitting the actual endpoint once it's using Diesel - req.db_conn().unwrap() - .execute("TRUNCATE TABLE follows") - .unwrap(); - // req.with_path("/api/v1/crates/foo_following/follow") - // .with_method(Method::Delete); - // let mut response = ok_resp!(middle.call(&mut req)); - // assert!(::json::(&mut response).ok); - // let mut response = ok_resp!(middle.call(&mut req)); - // assert!(::json::(&mut response).ok); - - // req.with_path("/api/v1/crates/foo_following/following") - // .with_method(Method::Get); - // let mut response = ok_resp!(middle.call(&mut req)); - // assert!(!::json::(&mut response).following); + req.with_path("/api/v1/crates/foo_following/follow") + .with_method(Method::Delete); + let mut response = ok_resp!(middle.call(&mut req)); + assert!(::json::(&mut response).ok); + let mut response = ok_resp!(middle.call(&mut req)); + assert!(::json::(&mut response).ok); + + req.with_path("/api/v1/crates/foo_following/following") + .with_method(Method::Get); + let mut response = ok_resp!(middle.call(&mut req)); + assert!(!::json::(&mut response).following); req.with_path("/api/v1/crates") .with_query("following=1") diff --git a/src/tests/user.rs b/src/tests/user.rs index eb75abf93dc..47a15141244 100644 --- a/src/tests/user.rs +++ b/src/tests/user.rs @@ -1,9 +1,12 @@ use conduit::{Handler, Method}; +use diesel::prelude::*; +use diesel::insert; use cargo_registry::Model; +use cargo_registry::db::RequestTransaction; use cargo_registry::krate::EncodableCrate; +use cargo_registry::schema::versions; use cargo_registry::user::{User, NewUser, EncodableUser}; -use cargo_registry::db::RequestTransaction; use cargo_registry::version::EncodableVersion; #[derive(RustcDecodable)] @@ -139,10 +142,27 @@ fn following() { #[derive(RustcDecodable)] struct Meta { more: bool } let (_b, app, middle) = ::app(); - let mut req = ::req(app, Method::Get, "/"); - ::mock_user(&mut req, ::user("foo")); - ::mock_crate(&mut req, ::krate("foo_fighters")); - ::mock_crate(&mut req, ::krate("bar_fighters")); + let mut req = ::req(app.clone(), Method::Get, "/"); + { + let conn = app.diesel_database.get().unwrap(); + let user = ::new_user("foo").create_or_update(&conn).unwrap(); + ::sign_in_as(&mut req, &user); + #[derive(Insertable)] + #[table_name="versions"] + struct NewVersion<'a> { + crate_id: i32, + num: &'a str, + } + let id1 = ::new_crate("foo_fighters").create_or_update(&conn, None, user.id) + .unwrap().id; + let id2 = ::new_crate("bar_fighters").create_or_update(&conn, None, user.id) + .unwrap().id; + let new_versions = vec![ + NewVersion { crate_id: id1, num: "1.0.0" }, + NewVersion { crate_id: id2, num: "1.0.0" }, + ]; + insert(&new_versions).into(versions::table).execute(&*conn).unwrap(); + } let mut response = ok_resp!(middle.call(req.with_path("/me/updates") .with_method(Method::Get))); diff --git a/src/user/mod.rs b/src/user/mod.rs index 3fc9cfc19df..52c3d314395 100644 --- a/src/user/mod.rs +++ b/src/user/mod.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use conduit::{Request, Response}; use conduit_cookie::{RequestSession}; @@ -11,19 +10,19 @@ use rand::{thread_rng, Rng}; use app::RequestApp; use db::RequestTransaction; -use {http, Model, Version}; -use krate::{Crate, EncodableCrate}; -use schema::users; +use krate::Follow; +use schema::*; use util::errors::NotFound; use util::{RequestUtils, CargoResult, internal, ChainError, human}; use version::EncodableVersion; +use {http, Model, Version}; pub use self::middleware::{Middleware, RequestUser}; pub mod middleware; /// The model representing a row in the `users` database table. -#[derive(Clone, Debug, PartialEq, Eq, Queryable)] +#[derive(Clone, Debug, PartialEq, Eq, Queryable, Identifiable)] pub struct User { pub id: i32, pub email: Option, @@ -356,62 +355,45 @@ pub fn show(req: &mut Request) -> CargoResult { /// Handles the `GET /me/updates` route. pub fn updates(req: &mut Request) -> CargoResult { + use diesel::expression::dsl::{any, sql}; + use diesel::types::BigInt; + let user = req.user()?; let (offset, limit) = req.pagination(10, 100)?; - let tx = req.tx()?; - let sql = "SELECT versions.* FROM versions - INNER JOIN follows - ON follows.user_id = $1 AND - follows.crate_id = versions.crate_id - ORDER BY versions.created_at DESC OFFSET $2 LIMIT $3"; - - // Load all versions - let stmt = tx.prepare(sql)?; - let mut versions = Vec::new(); - let mut crate_ids = Vec::new(); - for row in stmt.query(&[&user.id, &offset, &limit])?.iter() { - let version: Version = Model::from_row(&row); - crate_ids.push(version.crate_id); - versions.push(version); - } - - // Load all crates - let mut map = HashMap::new(); - let mut crates = Vec::new(); - if crate_ids.len() > 0 { - let stmt = tx.prepare("SELECT * FROM crates WHERE id = ANY($1)")?; - for row in stmt.query(&[&crate_ids])?.iter() { - let krate: Crate = Model::from_row(&row); - map.insert(krate.id, krate.name.clone()); - crates.push(krate); - } - } + let conn = req.db_conn()?; - // Encode everything! - let crates = crates.into_iter().map(|c| { - let max_version = c.max_version(tx)?; - Ok(c.minimal_encodable(max_version, None)) - }).collect::>()?; - let versions = versions.into_iter().map(|v| { - let id = v.crate_id; - v.encodable(&map[&id]) + let followed_crates = Follow::belonging_to(user) + .select(follows::crate_id); + let data = versions::table + .select(versions::id) // FIXME: Remove this line when upgraded to Diesel 0.12 + .inner_join(crates::table) + .filter(crates::id.eq(any(followed_crates))) + .order(versions::created_at.desc()) + .limit(limit) + .offset(offset) + .select(( + versions::all_columns, + crates::name, + sql::("COUNT(*) OVER ()"), + )) + .load::<(Version, String, i64)>(conn)?; + + let more = data.get(0) + .map(|&(_, _, count)| count > offset + limit) + .unwrap_or(false); + + let versions = data.into_iter().map(|(version, crate_name, _)| { + version.encodable(&crate_name) }).collect(); - // Check if we have another - let sql = format!("SELECT 1 WHERE EXISTS({})", sql); - let stmt = tx.prepare(&sql)?; - let more = stmt.query(&[&user.id, &(offset + limit), &limit])? - .iter().next().is_some(); - #[derive(RustcEncodable)] struct R { versions: Vec, - crates: Vec, meta: Meta, } #[derive(RustcEncodable)] struct Meta { more: bool } - Ok(req.json(&R{ versions: versions, crates: crates, meta: Meta { more: more } })) + Ok(req.json(&R{ versions: versions, meta: Meta { more: more } })) } #[cfg(test)]