Skip to content

Commit 119e80b

Browse files
alexcrichtonalexcrichton
committed
Merge pull request #270 from sgrif/sg-remove-keywords-column
Remove mostly unused keywords column from crates table
2 parents 978b106 + f973a15 commit 119e80b

File tree

6 files changed

+121
-55
lines changed

6 files changed

+121
-55
lines changed

‎src/bin/migrate.rs

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,7 @@ fn migrations() -> Vec<Migration> {
602602
try!(tx.batch_execute("
603603
DROP TRIGGER trigger_crates_set_updated_at ON crates;
604604
DROP TRIGGER trigger_versions_set_updated_at ON versions;
605-
DROP FUNCTION set_updated_at_ignore_downloads;
605+
DROP FUNCTION set_updated_at_ignore_downloads();
606606
CREATE TRIGGER trigger_crates_set_updated_at BEFORE UPDATE
607607
ON crates
608608
FOR EACH ROW EXECUTE PROCEDURE set_updated_at();
@@ -613,6 +613,75 @@ fn migrations() -> Vec<Migration> {
613613
"));
614614
Ok(())
615615
}),
616+
Migration::new(20160219125609, |tx| {
617+
tx.batch_execute("
618+
ALTER TABLE crates DROP COLUMN keywords;
619+
CREATE OR REPLACE FUNCTION trigger_crates_name_search() RETURNS trigger AS $$
620+
DECLARE kws TEXT;
621+
begin
622+
SELECT array_to_string(array_agg(keyword), ',') INTO kws
623+
FROM keywords INNER JOIN crates_keywords
624+
ON keywords.id = crates_keywords.keyword_id
625+
WHERE crates_keywords.crate_id = new.id;
626+
new.textsearchable_index_col :=
627+
setweight(to_tsvector('pg_catalog.english',
628+
coalesce(new.name, '')), 'A') ||
629+
setweight(to_tsvector('pg_catalog.english',
630+
coalesce(kws, '')), 'B') ||
631+
setweight(to_tsvector('pg_catalog.english',
632+
coalesce(new.description, '')), 'C') ||
633+
setweight(to_tsvector('pg_catalog.english',
634+
coalesce(new.readme, '')), 'D');
635+
return new;
636+
end
637+
$$ LANGUAGE plpgsql;
638+
CREATE OR REPLACE FUNCTION touch_crate() RETURNS trigger AS $$
639+
BEGIN
640+
IF TG_OP = 'DELETE' THEN
641+
UPDATE crates SET updated_at = CURRENT_TIMESTAMP WHERE
642+
id = OLD.crate_id;
643+
RETURN OLD;
644+
ELSE
645+
UPDATE crates SET updated_at = CURRENT_TIMESTAMP WHERE
646+
id = NEW.crate_id;
647+
RETURN NEW;
648+
END IF;
649+
END
650+
$$ LANGUAGE plpgsql;
651+
CREATE TRIGGER touch_crate_on_modify_keywords
652+
AFTER INSERT OR DELETE ON crates_keywords
653+
FOR EACH ROW
654+
EXECUTE PROCEDURE touch_crate();
655+
")
656+
}, |tx| {
657+
tx.batch_execute("
658+
ALTER TABLE crates ADD COLUMN keywords VARCHAR;
659+
CREATE OR REPLACE FUNCTION trigger_crates_name_search() RETURNS trigger AS $$
660+
begin
661+
new.textsearchable_index_col :=
662+
setweight(to_tsvector('pg_catalog.english',
663+
coalesce(new.name, '')), 'A') ||
664+
setweight(to_tsvector('pg_catalog.english',
665+
coalesce(new.keywords, '')), 'B') ||
666+
setweight(to_tsvector('pg_catalog.english',
667+
coalesce(new.description, '')), 'C') ||
668+
setweight(to_tsvector('pg_catalog.english',
669+
coalesce(new.readme, '')), 'D');
670+
return new;
671+
end
672+
$$ LANGUAGE plpgsql;
673+
674+
UPDATE crates SET keywords = (
675+
SELECT array_to_string(array_agg(keyword), ',')
676+
FROM keywords INNER JOIN crates_keywords
677+
ON keywords.id = crates_keywords.keyword_id
678+
WHERE crates_keywords.crate_id = crates.id
679+
);
680+
681+
DROP TRIGGER touch_crate_on_modify_keywords ON crates_keywords;
682+
DROP FUNCTION touch_crate();
683+
")
684+
}),
616685
];
617686
// NOTE: Generate a new id via `date +"%Y%m%d%H%M%S"`
618687

‎src/bin/update-downloads.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ mod test {
167167
let tx = conn.transaction().unwrap();
168168
let user = user(&tx);
169169
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None, &None,
170-
&None, &None, &[], &None, &None,
170+
&None, &None, &None, &None,
171171
&None, None).unwrap();
172172
let version = Version::insert(&tx, krate.id,
173173
&semver::Version::parse("1.0.0").unwrap(),
@@ -194,7 +194,7 @@ mod test {
194194
let tx = conn.transaction().unwrap();
195195
let user = user(&tx);
196196
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None,
197-
&None, &None, &None, &[], &None,
197+
&None, &None, &None, &None,
198198
&None, &None, None).unwrap();
199199
let version = Version::insert(&tx, krate.id,
200200
&semver::Version::parse("1.0.0").unwrap(),
@@ -217,7 +217,7 @@ mod test {
217217
let tx = conn.transaction().unwrap();
218218
let user = user(&tx);
219219
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None,
220-
&None, &None, &None, &[], &None,
220+
&None, &None, &None, &None,
221221
&None, &None, None).unwrap();
222222
let version = Version::insert(&tx, krate.id,
223223
&semver::Version::parse("1.0.0").unwrap(),
@@ -241,7 +241,7 @@ mod test {
241241
let tx = conn.transaction().unwrap();
242242
let user = user(&tx);
243243
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None,
244-
&None, &None, &None, &[], &None,
244+
&None, &None, &None, &None,
245245
&None, &None, None).unwrap();
246246
let version = Version::insert(&tx, krate.id,
247247
&semver::Version::parse("1.0.0").unwrap(),
@@ -281,7 +281,7 @@ mod test {
281281
let tx = conn.transaction().unwrap();
282282
let user = user(&tx);
283283
let krate = Crate::find_or_insert(&tx, "foo", user.id, &None,
284-
&None, &None, &None, &[], &None,
284+
&None, &None, &None, &None,
285285
&None, &None, None).unwrap();
286286
let version = Version::insert(&tx, krate.id,
287287
&semver::Version::parse("1.0.0").unwrap(),

‎src/krate.rs

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ pub struct Crate {
4848
pub homepage: Option<String>,
4949
pub documentation: Option<String>,
5050
pub readme: Option<String>,
51-
pub keywords: Vec<String>,
5251
pub license: Option<String>,
5352
pub repository: Option<String>,
5453
pub max_upload_size: Option<i32>,
@@ -66,7 +65,6 @@ pub struct EncodableCrate {
6665
pub description: Option<String>,
6766
pub homepage: Option<String>,
6867
pub documentation: Option<String>,
69-
pub keywords: Vec<String>,
7068
pub license: Option<String>,
7169
pub repository: Option<String>,
7270
pub links: CrateLinks,
@@ -99,7 +97,6 @@ impl Crate {
9997
homepage: &Option<String>,
10098
documentation: &Option<String>,
10199
readme: &Option<String>,
102-
keywords: &[String],
103100
repository: &Option<String>,
104101
license: &Option<String>,
105102
license_file: &Option<String>,
@@ -112,7 +109,6 @@ impl Crate {
112109
let repository = repository.as_ref().map(|s| &s[..]);
113110
let mut license = license.as_ref().map(|s| &s[..]);
114111
let license_file = license_file.as_ref().map(|s| &s[..]);
115-
let keywords = keywords.join(",");
116112
try!(validate_url(homepage, "homepage"));
117113
try!(validate_url(documentation, "documentation"));
118114
try!(validate_url(repository, "repository"));
@@ -138,14 +134,13 @@ impl Crate {
138134
homepage = $2,
139135
description = $3,
140136
readme = $4,
141-
keywords = $5,
142-
license = $6,
143-
repository = $7
137+
license = $5,
138+
repository = $6
144139
WHERE canon_crate_name(name) =
145-
canon_crate_name($8)
140+
canon_crate_name($7)
146141
RETURNING *"));
147142
let rows = try!(stmt.query(&[&documentation, &homepage,
148-
&description, &readme, &keywords,
143+
&description, &readme,
149144
&license, &repository,
150145
&name]));
151146
match rows.iter().next() {
@@ -162,13 +157,13 @@ impl Crate {
162157

163158
let stmt = try!(conn.prepare("INSERT INTO crates
164159
(name, user_id, description, homepage,
165-
documentation, readme, keywords,
160+
documentation, readme,
166161
repository, license, max_upload_size)
167-
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)
162+
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)
168163
RETURNING *"));
169164
let rows = try!(stmt.query(&[&name, &user_id,
170165
&description, &homepage,
171-
&documentation, &readme, &keywords,
166+
&documentation, &readme,
172167
&repository, &license, &max_upload_size]));
173168
let ret: Crate = Model::from_row(&try!(rows.iter().next().chain_error(|| {
174169
internal("no crate returned")
@@ -239,7 +234,7 @@ impl Crate {
239234
pub fn encodable(self, versions: Option<Vec<i32>>) -> EncodableCrate {
240235
let Crate {
241236
name, created_at, updated_at, downloads, max_version, description,
242-
homepage, documentation, keywords, license, repository,
237+
homepage, documentation, license, repository,
243238
readme: _, id: _, user_id: _, max_upload_size: _,
244239
} = self;
245240
let versions_link = match versions {
@@ -257,7 +252,6 @@ impl Crate {
257252
documentation: documentation,
258253
homepage: homepage,
259254
description: description,
260-
keywords: keywords,
261255
license: license,
262256
repository: repository,
263257
links: CrateLinks {
@@ -431,7 +425,6 @@ impl Crate {
431425
impl Model for Crate {
432426
fn from_row(row: &Row) -> Crate {
433427
let max: String = row.get("max_version");
434-
let kws: Option<String> = row.get("keywords");
435428
Crate {
436429
id: row.get("id"),
437430
name: row.get("name"),
@@ -444,9 +437,6 @@ impl Model for Crate {
444437
homepage: row.get("homepage"),
445438
readme: row.get("readme"),
446439
max_version: semver::Version::parse(&max).unwrap(),
447-
keywords: kws.unwrap_or(String::new()).split(',')
448-
.filter(|s| !s.is_empty())
449-
.map(|s| s.to_string()).collect(),
450440
license: row.get("license"),
451441
repository: row.get("repository"),
452442
max_upload_size: row.get("max_upload_size"),
@@ -666,7 +656,6 @@ pub fn new(req: &mut Request) -> CargoResult<Response> {
666656
&new_crate.homepage,
667657
&new_crate.documentation,
668658
&new_crate.readme,
669-
&keywords,
670659
&new_crate.repository,
671660
&new_crate.license,
672661
&new_crate.license_file,

‎src/tests/all.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ fn krate(name: &str) -> Crate {
191191
homepage: None,
192192
description: None,
193193
readme: None,
194-
keywords: Vec::new(),
195194
license: None,
196195
repository: None,
197196
max_upload_size: None,
@@ -222,13 +221,10 @@ fn mock_crate_vers(req: &mut Request, krate: Crate, v: &semver::Version)
222221
&krate.homepage,
223222
&krate.documentation,
224223
&krate.readme,
225-
&krate.keywords,
226224
&krate.repository,
227225
&krate.license,
228226
&None,
229227
krate.max_upload_size).unwrap();
230-
Keyword::update_crate(req.tx().unwrap(), &krate,
231-
&krate.keywords).unwrap();
232228
let v = krate.add_version(req.tx().unwrap(), v, &HashMap::new(), &[]);
233229
(krate, v.unwrap())
234230
}
@@ -260,13 +256,20 @@ fn new_req(app: Arc<App>, krate: &str, version: &str) -> MockRequest {
260256
fn new_req_full(app: Arc<App>, krate: Crate, version: &str,
261257
deps: Vec<u::CrateDependency>) -> MockRequest {
262258
let mut req = ::req(app, Method::Put, "/api/v1/crates/new");
263-
req.with_body(&new_req_body(krate, version, deps));
259+
req.with_body(&new_req_body(krate, version, deps, Vec::new()));
264260
return req;
265261
}
266262

267-
fn new_req_body(krate: Crate, version: &str, deps: Vec<u::CrateDependency>)
268-
-> Vec<u8> {
269-
let kws = krate.keywords.into_iter().map(u::Keyword).collect();
263+
fn new_req_with_keywords(app: Arc<App>, krate: Crate, version: &str,
264+
kws: Vec<String>) -> MockRequest {
265+
let mut req = ::req(app, Method::Put, "/api/v1/crates/new");
266+
req.with_body(&new_req_body(krate, version, Vec::new(), kws));
267+
return req;
268+
}
269+
270+
fn new_req_body(krate: Crate, version: &str, deps: Vec<u::CrateDependency>,
271+
kws: Vec<String>) -> Vec<u8> {
272+
let kws = kws.into_iter().map(u::Keyword).collect();
270273
new_crate_to_body(&u::NewCrate {
271274
name: u::CrateName(krate.name),
272275
vers: u::CrateVersion(semver::Version::parse(version).unwrap()),

‎src/tests/krate.rs

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,14 @@ use std::fs::{self, File};
44

55
use conduit::{Handler, Request, Method};
66
use git2;
7+
use postgres::GenericConnection;
78
use rustc_serialize::{json, Decoder};
89
use semver;
910

11+
use cargo_registry::db::RequestTransaction;
1012
use cargo_registry::dependency::EncodableDependency;
1113
use cargo_registry::download::EncodableVersionDownload;
14+
use cargo_registry::keyword::Keyword;
1215
use cargo_registry::krate::{Crate, EncodableCrate};
1316
use cargo_registry::upload as u;
1417
use cargo_registry::user::EncodableUser;
@@ -71,20 +74,22 @@ fn index() {
7174
assert_eq!(json.crates[0].id, krate.name);
7275
}
7376

77+
fn tx(req: &Request) -> &GenericConnection { req.tx().unwrap() }
78+
7479
#[test]
7580
fn index_queries() {
7681
let (_b, app, middle) = ::app();
7782

7883
let mut req = ::req(app, Method::Get, "/api/v1/crates");
7984
let u = ::mock_user(&mut req, ::user("foo"));
8085
let mut krate = ::krate("foo");
81-
krate.keywords.push("kw1".to_string());
8286
krate.readme = Some("readme".to_string());
8387
krate.description = Some("description".to_string());
84-
::mock_crate(&mut req, krate);
85-
let mut krate2 = ::krate("BAR");
86-
krate2.keywords.push("KW1".to_string());
87-
::mock_crate(&mut req, krate2);
88+
let (krate, _) = ::mock_crate(&mut req, krate.clone());
89+
let krate2 = ::krate("BAR");
90+
let (krate2, _) = ::mock_crate(&mut req, krate2.clone());
91+
Keyword::update_crate(tx(&req), &krate, &["kw1".into()]).unwrap();
92+
Keyword::update_crate(tx(&req), &krate2, &["KW1".into()]).unwrap();
8893

8994
let mut response = ok_resp!(middle.call(req.with_query("q=baz")));
9095
assert_eq!(::json::<CrateList>(&mut response).meta.total, 0);
@@ -345,7 +350,7 @@ fn new_crate_owner() {
345350
assert_eq!(::json::<CrateList>(&mut response).crates.len(), 1);
346351

347352
// And upload a new crate as the first user
348-
let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new());
353+
let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new());
349354
req.mut_extensions().insert(u2);
350355
let mut response = ok_resp!(middle.call(req.with_path("/api/v1/crates/new")
351356
.with_method(Method::Put)
@@ -734,33 +739,33 @@ fn yank_not_owner() {
734739
fn bad_keywords() {
735740
let (_b, app, middle) = ::app();
736741
{
737-
let mut krate = ::krate("foo");
738-
krate.keywords.push("super-long-keyword-name-oh-no".to_string());
739-
let mut req = ::new_req_full(app.clone(), krate, "1.0.0", Vec::new());
742+
let krate = ::krate("foo");
743+
let kws = vec!["super-long-keyword-name-oh-no".into()];
744+
let mut req = ::new_req_with_keywords(app.clone(), krate, "1.0.0", kws);
740745
::mock_user(&mut req, ::user("foo"));
741746
let mut response = ok_resp!(middle.call(&mut req));
742747
::json::<::Bad>(&mut response);
743748
}
744749
{
745-
let mut krate = ::krate("foo");
746-
krate.keywords.push("?@?%".to_string());
747-
let mut req = ::new_req_full(app.clone(), krate, "1.0.0", Vec::new());
750+
let krate = ::krate("foo");
751+
let kws = vec!["?@?%".into()];
752+
let mut req = ::new_req_with_keywords(app.clone(), krate, "1.0.0", kws);
748753
::mock_user(&mut req, ::user("foo"));
749754
let mut response = ok_resp!(middle.call(&mut req));
750755
::json::<::Bad>(&mut response);
751756
}
752757
{
753-
let mut krate = ::krate("foo");
754-
krate.keywords.push("?@?%".to_string());
755-
let mut req = ::new_req_full(app.clone(), krate, "1.0.0", Vec::new());
758+
let krate = ::krate("foo");
759+
let kws = vec!["?@?%".into()];
760+
let mut req = ::new_req_with_keywords(app.clone(), krate, "1.0.0", kws);
756761
::mock_user(&mut req, ::user("foo"));
757762
let mut response = ok_resp!(middle.call(&mut req));
758763
::json::<::Bad>(&mut response);
759764
}
760765
{
761-
let mut krate = ::krate("foo");
762-
krate.keywords.push("áccênts".to_string());
763-
let mut req = ::new_req_full(app.clone(), krate, "1.0.0", Vec::new());
766+
let krate = ::krate("foo");
767+
let kws = vec!["áccênts".into()];
768+
let mut req = ::new_req_with_keywords(app.clone(), krate, "1.0.0", kws);
764769
::mock_user(&mut req, ::user("foo"));
765770
let mut response = ok_resp!(middle.call(&mut req));
766771
::json::<::Bad>(&mut response);

0 commit comments

Comments
 (0)