Skip to content

Commit d5c5635

Browse files
committed
Port crates#index to use Diesel
I decided to tackle this one next since we have an N+1 queries bug on the max versions, it's a relatively high traffic endpoint, and the code benefitted the most from Diesel. There was probably more code added to support migrating the tests over than to support the actual `index` function, but all those pieces will start to get used as more endpoints are ported over. There were a few tests that were hitting the followings and owners endpoints which I have temporarily changed to hit the database directly instead. Porting those endpoints would mean porting over more tests, which might mean porting over more endpoints, etc. We didn't lose any coverage overall, but we should still go back to hitting the endpoints directly in tests when we can.
1 parent ddc630d commit d5c5635

File tree

18 files changed

+608
-261
lines changed

18 files changed

+608
-261
lines changed

Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,11 @@ rustc-serialize = "0.3"
3535
license-exprs = "^1.3"
3636
dotenv = "0.8.0"
3737
toml = "0.2"
38-
diesel = { version = "0.11.0", features = ["postgres", "serde_json"] }
38+
diesel = { version = "0.11.0", features = ["postgres", "serde_json", "deprecated-time"] }
3939
diesel_codegen = { version = "0.11.0", features = ["postgres"] }
4040
r2d2-diesel = "0.11.0"
4141
diesel_full_text_search = "0.11.0"
42+
serde_json = "0.9.0"
4243

4344
conduit = "0.8"
4445
conduit-conditional-get = "0.8"
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE versions ALTER COLUMN yanked DROP NOT NULL;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE versions ALTER COLUMN yanked SET NOT NULL;

src/app.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ impl App {
5454
.helper_threads(if config.env == ::Env::Production {3} else {1})
5555
.build();
5656
let diesel_db_config = r2d2::Config::builder()
57-
.pool_size(if config.env == ::Env::Production {1} else {1})
57+
.pool_size(if config.env == ::Env::Production {50} else {1})
58+
.min_idle(if config.env == ::Env::Production {Some(5)} else {None})
5859
.helper_threads(if config.env == ::Env::Production {3} else {1})
5960
.build();
6061

src/badge.rs

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
1-
use util::CargoResult;
2-
use krate::Crate;
31
use Model;
2+
use krate::Crate;
3+
use schema::badges;
4+
use util::CargoResult;
45

5-
use std::collections::HashMap;
6+
use diesel::pg::Pg;
7+
use diesel::prelude::*;
68
use pg::GenericConnection;
79
use pg::rows::Row;
810
use rustc_serialize::Decodable;
911
use rustc_serialize::json::{Json, Decoder};
12+
use serde_json;
13+
use std::collections::HashMap;
1014

1115
#[derive(Debug, PartialEq, Clone)]
1216
pub enum Badge {
@@ -27,6 +31,17 @@ pub struct EncodableBadge {
2731
pub attributes: HashMap<String, String>,
2832
}
2933

34+
impl Queryable<badges::SqlType, Pg> for Badge {
35+
type Row = (i32, String, serde_json::Value);
36+
37+
fn build((_, badge_type, attributes): Self::Row) -> Self {
38+
let attributes = serde_json::from_value::<HashMap<String, String>>(attributes)
39+
.expect("attributes was not a map in the database");
40+
Self::from_attributes(&badge_type, &attributes)
41+
.expect("invalid badge in the database")
42+
}
43+
}
44+
3045
impl Model for Badge {
3146
fn from_row(row: &Row) -> Badge {
3247
let attributes: Json = row.get("attributes");

src/category.rs

Lines changed: 64 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,37 @@ use time::Timespec;
33

44
use conduit::{Request, Response};
55
use conduit_router::RequestParams;
6+
use diesel::*;
7+
use diesel::pg::PgConnection;
68
use pg::GenericConnection;
79
use pg::rows::Row;
810

911
use {Model, Crate};
1012
use db::RequestTransaction;
13+
use schema::*;
1114
use util::{RequestUtils, CargoResult, ChainError};
1215
use util::errors::NotFound;
1316

14-
#[derive(Clone)]
17+
#[derive(Clone, Identifiable, Associations, Queryable)]
18+
#[has_many(crates_categories)]
19+
#[table_name="categories"]
1520
pub struct Category {
1621
pub id: i32,
1722
pub category: String,
1823
pub slug: String,
1924
pub description: String,
20-
pub created_at: Timespec,
2125
pub crates_cnt: i32,
26+
pub created_at: Timespec,
27+
}
28+
29+
#[derive(Associations, Insertable, Identifiable)]
30+
#[belongs_to(Category)]
31+
#[belongs_to(Crate)]
32+
#[table_name="crates_categories"]
33+
#[primary_key(crate_id, category_id)]
34+
struct CrateCategory {
35+
crate_id: i32,
36+
category_id: i32,
2237
}
2338

2439
#[derive(RustcEncodable, RustcDecodable)]
@@ -77,7 +92,27 @@ impl Category {
7792
}
7893
}
7994

80-
pub fn update_crate(conn: &GenericConnection,
95+
pub fn update_crate<'a>(conn: &PgConnection,
96+
krate: &Crate,
97+
slugs: &[&'a str]) -> QueryResult<Vec<&'a str>> {
98+
use diesel::expression::dsl::any;
99+
100+
let categories = categories::table
101+
.filter(categories::slug.eq(any(slugs)))
102+
.load::<Category>(conn)?;
103+
let invalid_categories = slugs.iter().cloned()
104+
.filter(|s| !categories.iter().any(|c| c.slug == *s))
105+
.collect();
106+
let crate_categories = categories.iter()
107+
.map(|c| CrateCategory { category_id: c.id, crate_id: krate.id })
108+
.collect::<Vec<_>>();
109+
110+
delete(CrateCategory::belonging_to(krate)).execute(conn)?;
111+
insert(&crate_categories).into(crates_categories::table).execute(conn)?;
112+
Ok(invalid_categories)
113+
}
114+
115+
pub fn update_crate_old(conn: &GenericConnection,
81116
krate: &Crate,
82117
categories: &[String]) -> CargoResult<Vec<String>> {
83118
let old_categories = krate.categories(conn)?;
@@ -194,6 +229,32 @@ impl Category {
194229
}
195230
}
196231

232+
#[derive(Insertable, Default)]
233+
#[table_name="categories"]
234+
pub struct NewCategory<'a> {
235+
pub category: &'a str,
236+
pub slug: &'a str,
237+
}
238+
239+
impl<'a> NewCategory<'a> {
240+
pub fn create_or_update(&self, conn: &PgConnection) -> QueryResult<Category> {
241+
use schema::categories::dsl::*;
242+
use diesel::pg::upsert::*;
243+
244+
let maybe_inserted = insert(&self.on_conflict_do_nothing())
245+
.into(categories)
246+
.get_result(conn)
247+
.optional()?;
248+
249+
if let Some(c) = maybe_inserted {
250+
return Ok(c);
251+
}
252+
253+
categories.filter(slug.eq(self.slug))
254+
.first(conn)
255+
}
256+
}
257+
197258
impl Model for Category {
198259
fn from_row(row: &Row) -> Category {
199260
Category {

src/keyword.rs

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,35 @@ use time::Timespec;
44

55
use conduit::{Request, Response};
66
use conduit_router::RequestParams;
7+
use diesel::pg::PgConnection;
8+
use diesel::prelude::*;
9+
use diesel;
710
use pg::GenericConnection;
811
use pg::rows::Row;
912

1013
use {Model, Crate};
1114
use db::RequestTransaction;
15+
use schema::*;
1216
use util::{RequestUtils, CargoResult, ChainError, internal};
1317
use util::errors::NotFound;
1418

15-
#[derive(Clone)]
19+
#[derive(Clone, Identifiable, Associations, Queryable)]
20+
#[has_many(crates_keywords)]
1621
pub struct Keyword {
1722
pub id: i32,
1823
pub keyword: String,
19-
pub created_at: Timespec,
2024
pub crates_cnt: i32,
25+
pub created_at: Timespec,
26+
}
27+
28+
#[derive(Associations, Insertable, Identifiable)]
29+
#[belongs_to(Keyword)]
30+
#[belongs_to(Crate)]
31+
#[table_name="crates_keywords"]
32+
#[primary_key(crate_id, keyword_id)]
33+
struct CrateKeyword {
34+
crate_id: i32,
35+
keyword_id: i32,
2136
}
2237

2338
#[derive(RustcEncodable, RustcDecodable)]
@@ -37,6 +52,27 @@ impl Keyword {
3752
Ok(rows.iter().next().map(|r| Model::from_row(&r)))
3853
}
3954

55+
pub fn find_all(conn: &PgConnection, names: &[&str]) -> QueryResult<Vec<Keyword>> {
56+
use diesel::pg::upsert::*;
57+
use diesel::expression::dsl::any;
58+
59+
#[derive(Insertable)]
60+
#[table_name="keywords"]
61+
struct NewKeyword<'a> {
62+
keyword: &'a str,
63+
}
64+
sql_function!(lower, lower_t, (x: ::diesel::types::Text) -> ::diesel::types::Text);
65+
66+
let (lowercase_names, new_keywords): (Vec<_>, Vec<_>) = names.iter()
67+
.map(|s| (s.to_lowercase(), NewKeyword { keyword: *s }))
68+
.unzip();
69+
70+
diesel::insert(&new_keywords.on_conflict_do_nothing()).into(keywords::table)
71+
.execute(conn)?;
72+
keywords::table.filter(lower(keywords::keyword).eq(any(lowercase_names)))
73+
.load(conn)
74+
}
75+
4076
pub fn find_or_insert(conn: &GenericConnection, name: &str)
4177
-> CargoResult<Keyword> {
4278
// TODO: racy (the select then insert is not atomic)
@@ -91,7 +127,21 @@ impl Keyword {
91127
}
92128
}
93129

94-
pub fn update_crate(conn: &GenericConnection,
130+
pub fn update_crate(conn: &PgConnection,
131+
krate: &Crate,
132+
keywords: &[&str]) -> QueryResult<()> {
133+
let keywords = Keyword::find_all(conn, keywords)?;
134+
diesel::delete(CrateKeyword::belonging_to(krate))
135+
.execute(conn)?;
136+
let crate_keywords = keywords.into_iter().map(|kw| {
137+
CrateKeyword { crate_id: krate.id, keyword_id: kw.id }
138+
}).collect::<Vec<_>>();
139+
diesel::insert(&crate_keywords).into(crates_keywords::table)
140+
.execute(conn)?;
141+
Ok(())
142+
}
143+
144+
pub fn update_crate_old(conn: &GenericConnection,
95145
krate: &Crate,
96146
keywords: &[String]) -> CargoResult<()> {
97147
let old_kws = krate.keywords(conn)?;

0 commit comments

Comments
 (0)