Skip to content

Commit defd894

Browse files
committed
Update keyword to use diesel
1 parent 170ec38 commit defd894

File tree

3 files changed

+81
-61
lines changed

3 files changed

+81
-61
lines changed

src/keyword.rs

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use {Model, Crate};
1414
use db::RequestTransaction;
1515
use schema::*;
1616
use util::{RequestUtils, CargoResult, ChainError, internal};
17-
use util::errors::NotFound;
1817

1918
#[derive(Clone, Identifiable, Queryable)]
2019
pub struct Keyword {
@@ -43,12 +42,11 @@ pub struct EncodableKeyword {
4342
}
4443

4544
impl Keyword {
46-
pub fn find_by_keyword(conn: &GenericConnection, name: &str)
47-
-> CargoResult<Option<Keyword>> {
48-
let stmt = conn.prepare("SELECT * FROM keywords \
49-
WHERE keyword = LOWER($1)")?;
50-
let rows = stmt.query(&[&name])?;
51-
Ok(rows.iter().next().map(|r| Model::from_row(&r)))
45+
pub fn find_by_keyword(conn: &PgConnection, name: &str)
46+
-> QueryResult<Keyword> {
47+
keywords::table
48+
.filter(keywords::keyword.eq(::lower(name)))
49+
.first(&*conn)
5250
}
5351

5452
pub fn find_or_create_all(conn: &PgConnection, names: &[&str]) -> QueryResult<Vec<Keyword>> {
@@ -91,26 +89,6 @@ impl Keyword {
9189
})?))
9290
}
9391

94-
pub fn all(conn: &GenericConnection, sort: &str, limit: i64, offset: i64)
95-
-> CargoResult<Vec<Keyword>> {
96-
97-
let sort_sql = match sort {
98-
"crates" => "ORDER BY crates_cnt DESC",
99-
_ => "ORDER BY keyword ASC",
100-
};
101-
102-
let stmt = conn.prepare(&format!("SELECT * FROM keywords {}
103-
LIMIT $1 OFFSET $2",
104-
sort_sql))?;
105-
106-
let keywords: Vec<_> = stmt.query(&[&limit, &offset])?
107-
.iter()
108-
.map(|row| Model::from_row(&row))
109-
.collect();
110-
111-
Ok(keywords)
112-
}
113-
11492
pub fn valid_name(name: &str) -> bool {
11593
if name.is_empty() { return false }
11694
name.chars().next().unwrap().is_alphanumeric() &&
@@ -200,34 +178,49 @@ impl Model for Keyword {
200178

201179
/// Handles the `GET /keywords` route.
202180
pub fn index(req: &mut Request) -> CargoResult<Response> {
203-
let conn = req.tx()?;
181+
use diesel::expression::dsl::sql;
182+
use diesel::types::BigInt;
183+
use schema::keywords;
184+
185+
let conn = req.db_conn()?;
204186
let (offset, limit) = req.pagination(10, 100)?;
205187
let query = req.query();
206188
let sort = query.get("sort").map(|s| &s[..]).unwrap_or("alpha");
207189

208-
let keywords = Keyword::all(conn, sort, limit, offset)?;
209-
let keywords = keywords.into_iter().map(Keyword::encodable).collect();
190+
let mut query = keywords::table
191+
.select((keywords::all_columns, sql::<BigInt>("COUNT(*) OVER ()")))
192+
.limit(limit)
193+
.offset(offset)
194+
.into_boxed();
210195

211-
// Query for the total count of keywords
212-
let total = Keyword::count(conn)?;
196+
if sort == "crates" {
197+
query = query.order(keywords::crates_cnt.desc());
198+
} else {
199+
query = query.order(keywords::keyword.asc());
200+
}
201+
202+
let data = query.load::<(Keyword, i64)>(&*conn)?;
203+
let total = data.get(0).map(|&(_, t)| t).unwrap_or(0);
204+
let kws = data.into_iter()
205+
.map(|(k, _)| k.encodable()).collect::<Vec<_>>();
213206

214207
#[derive(RustcEncodable)]
215208
struct R { keywords: Vec<EncodableKeyword>, meta: Meta }
216209
#[derive(RustcEncodable)]
217210
struct Meta { total: i64 }
218211

219212
Ok(req.json(&R {
220-
keywords: keywords,
213+
keywords: kws,
221214
meta: Meta { total: total },
222215
}))
223216
}
224217

225218
/// Handles the `GET /keywords/:keyword_id` route.
226219
pub fn show(req: &mut Request) -> CargoResult<Response> {
227220
let name = &req.params()["keyword_id"];
228-
let conn = req.tx()?;
229-
let kw = Keyword::find_by_keyword(conn, name)?;
230-
let kw = kw.chain_error(|| NotFound)?;
221+
let conn = req.db_conn()?;
222+
223+
let kw = Keyword::find_by_keyword(&conn, name)?;
231224

232225
#[derive(RustcEncodable)]
233226
struct R { keyword: EncodableKeyword }

src/tests/all.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ use cargo_registry::krate::NewCrate;
3232
use cargo_registry::upload as u;
3333
use cargo_registry::user::NewUser;
3434
use cargo_registry::version::NewVersion;
35-
use cargo_registry::{User, Crate, Version, Keyword, Dependency, Category, Model, Replica};
35+
use cargo_registry::{User, Crate, Version, Dependency, Category, Model, Replica};
3636
use conduit::{Request, Method};
3737
use conduit_test::MockRequest;
3838
use diesel::pg::PgConnection;
@@ -296,10 +296,6 @@ fn mock_dep(req: &mut Request, version: &Version, krate: &Crate,
296296
&target.map(|s| s.to_string())).unwrap()
297297
}
298298

299-
fn mock_keyword(req: &mut Request, name: &str) -> Keyword {
300-
Keyword::find_or_insert(req.tx().unwrap(), name).unwrap()
301-
}
302-
303299
fn new_category<'a>(category: &'a str, slug: &'a str) -> NewCategory<'a> {
304300
NewCategory { category: category, slug: slug, ..NewCategory::default() }
305301
}

src/tests/keyword.rs

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use conduit::{Handler, Method};
22
use conduit_test::MockRequest;
33

4-
use cargo_registry::db::RequestTransaction;
54
use cargo_registry::keyword::{Keyword, EncodableKeyword};
65

76
#[derive(RustcDecodable)]
@@ -14,13 +13,16 @@ struct GoodKeyword { keyword: EncodableKeyword }
1413
#[test]
1514
fn index() {
1615
let (_b, app, middle) = ::app();
17-
let mut req = ::req(app, Method::Get, "/api/v1/keywords");
16+
let mut req = ::req(app.clone(), Method::Get, "/api/v1/keywords");
1817
let mut response = ok_resp!(middle.call(&mut req));
1918
let json: KeywordList = ::json(&mut response);
2019
assert_eq!(json.keywords.len(), 0);
2120
assert_eq!(json.meta.total, 0);
2221

23-
::mock_keyword(&mut req, "foo");
22+
{
23+
let conn = app.diesel_database.get().unwrap();
24+
Keyword::find_or_create_all(&conn, &["foo"]).unwrap();
25+
}
2426
let mut response = ok_resp!(middle.call(&mut req));
2527
let json: KeywordList = ::json(&mut response);
2628
assert_eq!(json.keywords.len(), 1);
@@ -31,11 +33,14 @@ fn index() {
3133
#[test]
3234
fn show() {
3335
let (_b, app, middle) = ::app();
34-
let mut req = ::req(app, Method::Get, "/api/v1/keywords/foo");
36+
let mut req = ::req(app.clone(), Method::Get, "/api/v1/keywords/foo");
3537
let response = t_resp!(middle.call(&mut req));
3638
assert_eq!(response.status.0, 404);
3739

38-
::mock_keyword(&mut req, "foo");
40+
{
41+
let conn = app.diesel_database.get().unwrap();
42+
Keyword::find_or_create_all(&conn, &["foo"]).unwrap();
43+
}
3944
let mut response = ok_resp!(middle.call(&mut req));
4045
let json: GoodKeyword = ::json(&mut response);
4146
assert_eq!(json.keyword.keyword, "foo".to_string());
@@ -44,8 +49,11 @@ fn show() {
4449
#[test]
4550
fn uppercase() {
4651
let (_b, app, middle) = ::app();
47-
let mut req = ::req(app, Method::Get, "/api/v1/keywords/UPPER");
48-
::mock_keyword(&mut req, "UPPER");
52+
let mut req = ::req(app.clone(), Method::Get, "/api/v1/keywords/UPPER");
53+
{
54+
let conn = app.diesel_database.get().unwrap();
55+
Keyword::find_or_create_all(&conn, &["UPPER"]).unwrap();
56+
}
4957

5058
let mut res = ok_resp!(middle.call(&mut req));
5159
let json: GoodKeyword = ::json(&mut res);
@@ -55,40 +63,63 @@ fn uppercase() {
5563
#[test]
5664
fn update_crate() {
5765
let (_b, app, middle) = ::app();
58-
let mut req = ::req(app, Method::Get, "/api/v1/keywords/foo");
66+
let mut req = ::req(app.clone(), Method::Get, "/api/v1/keywords/foo");
5967
let cnt = |req: &mut MockRequest, kw: &str| {
6068
req.with_path(&format!("/api/v1/keywords/{}", kw));
6169
let mut response = ok_resp!(middle.call(req));
6270
::json::<GoodKeyword>(&mut response).keyword.crates_cnt as usize
6371
};
64-
::mock_user(&mut req, ::user("foo"));
65-
let (krate, _) = ::mock_crate(&mut req, ::krate("fookey"));
66-
::mock_keyword(&mut req, "kw1");
67-
::mock_keyword(&mut req, "kw2");
6872

69-
Keyword::update_crate_old(req.tx().unwrap(), &krate, &[]).unwrap();
73+
let krate = {
74+
let conn = app.diesel_database.get().unwrap();
75+
let u = ::new_user("foo")
76+
.create_or_update(&conn)
77+
.unwrap();
78+
Keyword::find_or_create_all(&conn, &["kw1", "kw2"]).unwrap();
79+
::new_crate("fookey")
80+
.create_or_update(&conn, None, u.id)
81+
.unwrap()
82+
};
83+
84+
{
85+
let conn = app.diesel_database.get().unwrap();
86+
Keyword::update_crate(&conn, &krate, &[]).unwrap();
87+
}
7088
assert_eq!(cnt(&mut req, "kw1"), 0);
7189
assert_eq!(cnt(&mut req, "kw2"), 0);
7290

73-
Keyword::update_crate_old(req.tx().unwrap(), &krate, &["kw1".to_string()]).unwrap();
91+
{
92+
let conn = app.diesel_database.get().unwrap();
93+
Keyword::update_crate(&conn, &krate, &["kw1"]).unwrap();
94+
}
7495
assert_eq!(cnt(&mut req, "kw1"), 1);
7596
assert_eq!(cnt(&mut req, "kw2"), 0);
7697

77-
Keyword::update_crate_old(req.tx().unwrap(), &krate, &["kw2".to_string()]).unwrap();
98+
{
99+
let conn = app.diesel_database.get().unwrap();
100+
Keyword::update_crate(&conn, &krate, &["kw2"]).unwrap();
101+
}
78102
assert_eq!(cnt(&mut req, "kw1"), 0);
79103
assert_eq!(cnt(&mut req, "kw2"), 1);
80104

81-
Keyword::update_crate_old(req.tx().unwrap(), &krate, &[]).unwrap();
105+
{
106+
let conn = app.diesel_database.get().unwrap();
107+
Keyword::update_crate(&conn, &krate, &[]).unwrap();
108+
}
82109
assert_eq!(cnt(&mut req, "kw1"), 0);
83110
assert_eq!(cnt(&mut req, "kw2"), 0);
84111

85-
Keyword::update_crate_old(req.tx().unwrap(), &krate, &["kw1".to_string(),
86-
"kw2".to_string()]).unwrap();
112+
{
113+
let conn = app.diesel_database.get().unwrap();
114+
Keyword::update_crate(&conn, &krate, &["kw1", "kw2"]).unwrap();
115+
}
87116
assert_eq!(cnt(&mut req, "kw1"), 1);
88117
assert_eq!(cnt(&mut req, "kw2"), 1);
89118

90-
Keyword::update_crate_old(req.tx().unwrap(), &krate, &[]).unwrap();
119+
{
120+
let conn = app.diesel_database.get().unwrap();
121+
Keyword::update_crate(&conn, &krate, &[]).unwrap();
122+
}
91123
assert_eq!(cnt(&mut req, "kw1"), 0);
92124
assert_eq!(cnt(&mut req, "kw2"), 0);
93-
94125
}

0 commit comments

Comments
 (0)