From e18b2f2c95ebfe5e0bc1e79abfc7207eddd61aca Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Tue, 15 Nov 2016 11:29:34 -0500 Subject: [PATCH 01/34] Add migrations for storing categories --- src/bin/migrate.rs | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/bin/migrate.rs b/src/bin/migrate.rs index 89a62b52897..0b6d76121e8 100644 --- a/src/bin/migrate.rs +++ b/src/bin/migrate.rs @@ -764,6 +764,61 @@ fn migrations() -> Vec { try!(tx.execute("DROP INDEX users_gh_id", &[])); Ok(()) }), + Migration::add_table(20161115110541, "categories", " \ + id SERIAL PRIMARY KEY, \ + category VARCHAR NOT NULL UNIQUE, \ + crates_cnt INTEGER NOT NULL DEFAULT 0, \ + created_at TIMESTAMP NOT NULL DEFAULT current_timestamp"), + Migration::add_table(20161115111828, "crates_categories", " \ + crate_id INTEGER NOT NULL, \ + category_id INTEGER NOT NULL"), + foreign_key(20161115111836, "crates_categories", "crate_id", "crates (id)"), + Migration::run(20161115111846, " \ + ALTER TABLE crates_categories \ + ADD CONSTRAINT fk_crates_categories_category_id \ + FOREIGN KEY (category_id) REFERENCES categories (id) \ + ON DELETE CASCADE", " \ + ALTER TABLE crates_categories \ + DROP CONSTRAINT fk_crates_categories_category_id"), + index(20161115111853, "crates_categories", "crate_id"), + index(20161115111900, "crates_categories", "category_id"), + Migration::new(20161115111957, |tx| { + try!(tx.batch_execute(" \ + CREATE FUNCTION update_categories_crates_cnt() \ + RETURNS trigger AS $$ \ + BEGIN \ + IF (TG_OP = 'INSERT') THEN \ + UPDATE categories \ + SET crates_cnt = crates_cnt + 1 \ + WHERE id = NEW.category_id; \ + return NEW; \ + ELSIF (TG_OP = 'DELETE') THEN \ + UPDATE categories \ + SET crates_cnt = crates_cnt - 1 \ + WHERE id = OLD.category_id; \ + return OLD; \ + END IF; \ + END \ + $$ LANGUAGE plpgsql; \ + CREATE TRIGGER trigger_update_categories_crates_cnt \ + BEFORE INSERT OR DELETE \ + ON crates_categories \ + FOR EACH ROW EXECUTE PROCEDURE update_categories_crates_cnt(); \ + CREATE TRIGGER touch_crate_on_modify_categories \ + AFTER INSERT OR DELETE ON crates_categories \ + FOR EACH ROW \ + EXECUTE PROCEDURE touch_crate(); \ + ")); + Ok(()) + }, |tx| { + try!(tx.batch_execute(" \ + DROP TRIGGER trigger_update_categories_crates_cnt \ + ON crates_categories; \ + DROP FUNCTION update_categories_crates_cnt(); \ + DROP TRIGGER touch_crate_on_modify_categories \ + ON crates_categories;")); + Ok(()) + }), ]; // NOTE: Generate a new id via `date +"%Y%m%d%H%M%S"` From 7893307cd321a99bee6bbf33ded8bbfbb1abee2e Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Tue, 15 Nov 2016 16:43:52 -0500 Subject: [PATCH 02/34] Create a binary that syncs categories from a text file --- src/bin/sync-categories.rs | 48 ++++++++++++++++++++++++++++++++++++++ src/categories.txt | 2 ++ 2 files changed, 50 insertions(+) create mode 100644 src/bin/sync-categories.rs create mode 100644 src/categories.txt diff --git a/src/bin/sync-categories.rs b/src/bin/sync-categories.rs new file mode 100644 index 00000000000..86b68daea9a --- /dev/null +++ b/src/bin/sync-categories.rs @@ -0,0 +1,48 @@ +// Sync available crate categories from `src/categories.txt`. +// Only needs to be run for new databases or when `src/categories.txt` +// has been updated. +// +// Usage: +// cargo run --bin sync-categories + +#![deny(warnings)] + +extern crate cargo_registry; +extern crate postgres; + +use cargo_registry::env; + +fn main() { + let conn = postgres::Connection::connect(&env("DATABASE_URL")[..], + postgres::TlsMode::None).unwrap(); + let tx = conn.transaction().unwrap(); + sync(&tx).unwrap(); + tx.set_commit(); + tx.finish().unwrap(); +} + +fn sync(tx: &postgres::transaction::Transaction) -> postgres::Result<()> { + let categories = include_str!("../categories.txt"); + let categories: Vec<_> = categories.lines().collect(); + let insert = categories.iter() + .map(|c| format!("('{}')", c)) + .collect::>() + .join(","); + let in_clause = categories.iter() + .map(|c| format!("'{}'", c)) + .collect::>() + .join(","); + + try!(tx.batch_execute( + &format!(" \ + INSERT INTO categories (category) \ + VALUES {} \ + ON CONFLICT (category) DO NOTHING; \ + DELETE FROM categories \ + WHERE category NOT IN ({});", + insert, + in_clause + )[..] + )); + Ok(()) +} diff --git a/src/categories.txt b/src/categories.txt new file mode 100644 index 00000000000..2447433817a --- /dev/null +++ b/src/categories.txt @@ -0,0 +1,2 @@ +Test +Attempt From 3854dfcbb258c2d424000743a1fa8fe574f3c5ba Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 16 Nov 2016 15:31:13 -0500 Subject: [PATCH 03/34] Add a page that lists all categories --- app/controllers/categories.js | 17 +++++++ app/models/category.js | 9 ++++ app/router.js | 1 + app/routes/categories.js | 12 +++++ app/templates/categories.hbs | 66 ++++++++++++++++++++++++ src/category.rs | 95 +++++++++++++++++++++++++++++++++++ src/lib.rs | 3 ++ src/tests/all.rs | 14 +++++- src/tests/category.rs | 25 +++++++++ 9 files changed, 240 insertions(+), 2 deletions(-) create mode 100644 app/controllers/categories.js create mode 100644 app/models/category.js create mode 100644 app/routes/categories.js create mode 100644 app/templates/categories.hbs create mode 100644 src/category.rs create mode 100644 src/tests/category.rs diff --git a/app/controllers/categories.js b/app/controllers/categories.js new file mode 100644 index 00000000000..5cfc81b0f86 --- /dev/null +++ b/app/controllers/categories.js @@ -0,0 +1,17 @@ +import Ember from 'ember'; +import PaginationMixin from '../mixins/pagination'; + +const { computed } = Ember; + +export default Ember.Controller.extend(PaginationMixin, { + queryParams: ['page', 'per_page', 'sort'], + page: '1', + per_page: 10, + sort: 'alpha', + + totalItems: computed.readOnly('model.meta.total'), + + currentSortBy: computed('sort', function() { + return (this.get('sort') === 'crates') ? '# Crates' : 'Alphabetical'; + }), +}); diff --git a/app/models/category.js b/app/models/category.js new file mode 100644 index 00000000000..6a950224a9c --- /dev/null +++ b/app/models/category.js @@ -0,0 +1,9 @@ +import DS from 'ember-data'; + +export default DS.Model.extend({ + category: DS.attr('string'), + created_at: DS.attr('date'), + crates_cnt: DS.attr('number'), + + crates: DS.hasMany('crate', { async: true }) +}); diff --git a/app/router.js b/app/router.js index 8e48c6c8ca7..62826eeb499 100644 --- a/app/router.js +++ b/app/router.js @@ -35,6 +35,7 @@ Router.map(function() { this.route('keyword', { path: '/keywords/:keyword_id' }, function() { this.route('index', { path: '/' }); }); + this.route('categories'); this.route('catchAll', { path: '*path' }); }); diff --git a/app/routes/categories.js b/app/routes/categories.js new file mode 100644 index 00000000000..a3b07940be1 --- /dev/null +++ b/app/routes/categories.js @@ -0,0 +1,12 @@ +import Ember from 'ember'; + +export default Ember.Route.extend({ + queryParams: { + page: { refreshModel: true }, + sort: { refreshModel: true }, + }, + + model(params) { + return this.store.query('category', params); + }, +}); diff --git a/app/templates/categories.hbs b/app/templates/categories.hbs new file mode 100644 index 00000000000..d1fe45f3c6e --- /dev/null +++ b/app/templates/categories.hbs @@ -0,0 +1,66 @@ +{{ title 'Categories' }} + +
+ +

All Categories

+
+ +
+ + +
+ Sort by + {{#rl-dropdown-container class="dropdown-container"}} + {{#rl-dropdown-toggle tagName="a" class="dropdown"}} + + {{ currentSortBy }} + + {{/rl-dropdown-toggle}} + + {{#rl-dropdown tagName="ul" class="dropdown" closeOnChildClick="a:link"}} +
  • + {{#link-to (query-params sort="alpha")}} + Alphabetical + {{/link-to}} +
  • +
  • + {{#link-to (query-params sort="crates")}} + # Crates + {{/link-to}} +
  • + {{/rl-dropdown}} + {{/rl-dropdown-container}} +
    +
    + +
    + {{#each model as |category|}} +
    +
    + {{ category.category }} + + {{ format-num category.crates_cnt }} + crates + +
    +
    + {{/each}} +
    + + diff --git a/src/category.rs b/src/category.rs new file mode 100644 index 00000000000..c2f75c887c6 --- /dev/null +++ b/src/category.rs @@ -0,0 +1,95 @@ +use time::Timespec; + +use conduit::{Request, Response}; +use pg::GenericConnection; +use pg::rows::Row; + +use Model; +use db::RequestTransaction; +use util::{RequestUtils, CargoResult}; + +#[derive(Clone)] +pub struct Category { + pub id: i32, + pub category: String, + pub created_at: Timespec, + pub crates_cnt: i32, +} + +#[derive(RustcEncodable, RustcDecodable)] +pub struct EncodableCategory { + pub id: String, + pub category: String, + pub created_at: String, + pub crates_cnt: i32, +} + +impl Category { + pub fn find_by_category(conn: &GenericConnection, name: &str) + -> CargoResult> { + let stmt = try!(conn.prepare("SELECT * FROM categories \ + WHERE category = $1")); + let rows = try!(stmt.query(&[&name])); + Ok(rows.iter().next().map(|r| Model::from_row(&r))) + } + + pub fn encodable(self) -> EncodableCategory { + let Category { id: _, crates_cnt, category, created_at } = self; + EncodableCategory { + id: category.clone(), + created_at: ::encode_time(created_at), + crates_cnt: crates_cnt, + category: category, + } + } +} + +impl Model for Category { + fn from_row(row: &Row) -> Category { + Category { + id: row.get("id"), + created_at: row.get("created_at"), + crates_cnt: row.get("crates_cnt"), + category: row.get("category"), + } + } + fn table_name(_: Option) -> &'static str { "categories" } +} + +/// Handles the `GET /categories` route. +pub fn index(req: &mut Request) -> CargoResult { + let conn = try!(req.tx()); + let (offset, limit) = try!(req.pagination(10, 100)); + let query = req.query(); + let sort = query.get("sort").map(|s| &s[..]).unwrap_or("alpha"); + let sort_sql = match sort { + "crates" => "ORDER BY crates_cnt DESC", + _ => "ORDER BY category ASC", + }; + + // Collect all the categories + let stmt = try!(conn.prepare(&format!("SELECT * FROM categories {} \ + LIMIT $1 OFFSET $2", + sort_sql))); + + let categories: Vec<_> = try!(stmt.query(&[&limit, &offset])) + .iter() + .map(|row| { + let category: Category = Model::from_row(&row); + category.encodable() + }) + .collect(); + + // Query for the total count of categories + let total = try!(Category::count(conn)); + + #[derive(RustcEncodable)] + struct R { categories: Vec, meta: Meta } + #[derive(RustcEncodable)] + struct Meta { total: i64 } + + Ok(req.json(&R { + categories: categories, + meta: Meta { total: total }, + })) +} diff --git a/src/lib.rs b/src/lib.rs index 33596374d86..03385bed850 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,6 +33,7 @@ extern crate conduit_router; extern crate conduit_static; pub use app::App; +pub use self::category::Category; pub use config::Config; pub use self::dependency::Dependency; pub use self::download::{CrateDownload, VersionDownload}; @@ -51,6 +52,7 @@ use conduit_middleware::MiddlewareBuilder; use util::{C, R, R404}; pub mod app; +pub mod category; pub mod config; pub mod db; pub mod dependency; @@ -100,6 +102,7 @@ pub fn middleware(app: Arc) -> MiddlewareBuilder { api_router.get("/versions/:version_id", C(version::show)); api_router.get("/keywords", C(keyword::index)); api_router.get("/keywords/:keyword_id", C(keyword::show)); + api_router.get("/categories", C(category::index)); api_router.get("/users/:user_id", C(user::show)); let api_router = Arc::new(R404(api_router)); diff --git a/src/tests/all.rs b/src/tests/all.rs index 784e46ec0d4..f01eee904ef 100755 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -26,7 +26,7 @@ use conduit_test::MockRequest; use cargo_registry::app::App; use cargo_registry::db::{self, RequestTransaction}; use cargo_registry::dependency::Kind; -use cargo_registry::{User, Crate, Version, Keyword, Dependency}; +use cargo_registry::{User, Crate, Version, Keyword, Dependency, Category, Model}; use cargo_registry::upload as u; macro_rules! t { @@ -63,6 +63,7 @@ struct Error { detail: String } #[derive(RustcDecodable)] struct Bad { errors: Vec } +mod category; mod keyword; mod krate; mod user; @@ -250,11 +251,20 @@ fn mock_keyword(req: &mut Request, name: &str) -> Keyword { Keyword::find_or_insert(req.tx().unwrap(), name).unwrap() } +fn mock_category(req: &mut Request, name: &str) -> Category { + let conn = req.tx().unwrap(); + let stmt = conn.prepare(" \ + INSERT INTO categories (category) \ + VALUES ($1) \ + RETURNING *").unwrap(); + let rows = stmt.query(&[&name]).unwrap(); + Model::from_row(&rows.iter().next().unwrap()) +} + fn logout(req: &mut Request) { req.mut_extensions().pop::(); } - fn new_req(app: Arc, krate: &str, version: &str) -> MockRequest { new_req_full(app, ::krate(krate), version, Vec::new()) } diff --git a/src/tests/category.rs b/src/tests/category.rs new file mode 100644 index 00000000000..fb8fb7743e0 --- /dev/null +++ b/src/tests/category.rs @@ -0,0 +1,25 @@ +use conduit::{Handler, Method}; + +use cargo_registry::category::EncodableCategory; + +#[derive(RustcDecodable)] +struct CategoryList { categories: Vec, meta: CategoryMeta } +#[derive(RustcDecodable)] +struct CategoryMeta { total: i32 } + +#[test] +fn index() { + let (_b, app, middle) = ::app(); + let mut req = ::req(app, Method::Get, "/api/v1/categories"); + let mut response = ok_resp!(middle.call(&mut req)); + let json: CategoryList = ::json(&mut response); + assert_eq!(json.categories.len(), 0); + assert_eq!(json.meta.total, 0); + + ::mock_category(&mut req, "foo"); + let mut response = ok_resp!(middle.call(&mut req)); + let json: CategoryList = ::json(&mut response); + assert_eq!(json.categories.len(), 1); + assert_eq!(json.meta.total, 1); + assert_eq!(json.categories[0].category, "foo"); +} From 55f201fc849fa57356d22b1777bad82d826e6426 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 16 Nov 2016 16:26:22 -0500 Subject: [PATCH 04/34] Add a page that lists the crates in a category --- app/controllers/category/index.js | 17 +++++++++ app/router.js | 3 ++ app/routes/category.js | 11 ++++++ app/routes/category/index.js | 18 ++++++++++ app/templates/categories.hbs | 2 +- app/templates/category/error.hbs | 1 + app/templates/category/index.hbs | 59 +++++++++++++++++++++++++++++++ src/category.rs | 16 ++++++++- src/krate.rs | 13 +++++++ src/lib.rs | 1 + src/tests/category.rs | 15 ++++++++ 11 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 app/controllers/category/index.js create mode 100644 app/routes/category.js create mode 100644 app/routes/category/index.js create mode 100644 app/templates/category/error.hbs create mode 100644 app/templates/category/index.hbs diff --git a/app/controllers/category/index.js b/app/controllers/category/index.js new file mode 100644 index 00000000000..f1845376018 --- /dev/null +++ b/app/controllers/category/index.js @@ -0,0 +1,17 @@ +import Ember from 'ember'; +import PaginationMixin from '../../mixins/pagination'; + +const { computed } = Ember; + +export default Ember.Controller.extend(PaginationMixin, { + queryParams: ['page', 'per_page', 'sort'], + page: '1', + per_page: 10, + sort: 'alpha', + + totalItems: computed.readOnly('model.meta.total'), + + currentSortBy: computed('sort', function() { + return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; + }), +}); diff --git a/app/router.js b/app/router.js index 62826eeb499..e65d3ea9a4f 100644 --- a/app/router.js +++ b/app/router.js @@ -36,6 +36,9 @@ Router.map(function() { this.route('index', { path: '/' }); }); this.route('categories'); + this.route('category', { path: '/categories/:category_id' }, function() { + this.route('index', { path: '/' }); + }); this.route('catchAll', { path: '*path' }); }); diff --git a/app/routes/category.js b/app/routes/category.js new file mode 100644 index 00000000000..a9ca79343de --- /dev/null +++ b/app/routes/category.js @@ -0,0 +1,11 @@ +import Ember from 'ember'; + +export default Ember.Route.extend({ + model(params) { + return this.store.find('category', params.category_id).catch(e => { + if (e.errors.any(e => e.detail === 'Not Found')) { + this.controllerFor('application').set('flashError', `Category '${params.category_id}' does not exist`); + } + }); + } +}); diff --git a/app/routes/category/index.js b/app/routes/category/index.js new file mode 100644 index 00000000000..fd16edd91d2 --- /dev/null +++ b/app/routes/category/index.js @@ -0,0 +1,18 @@ +import Ember from 'ember'; + +export default Ember.Route.extend({ + queryParams: { + page: { refreshModel: true }, + sort: { refreshModel: true }, + }, + + model(params) { + params.category = this.modelFor('category').id; + return this.store.query('crate', params); + }, + + setupController(controller, model) { + controller.set('category', this.modelFor('category')); + this._super(controller, model); + }, +}); diff --git a/app/templates/categories.hbs b/app/templates/categories.hbs index d1fe45f3c6e..71cd2d8d21f 100644 --- a/app/templates/categories.hbs +++ b/app/templates/categories.hbs @@ -43,7 +43,7 @@ {{#each model as |category|}}
    - {{ category.category }} + {{link-to category.id "category" category}} {{ format-num category.crates_cnt }} crates diff --git a/app/templates/category/error.hbs b/app/templates/category/error.hbs new file mode 100644 index 00000000000..d3e3b5bbccc --- /dev/null +++ b/app/templates/category/error.hbs @@ -0,0 +1 @@ +{{ title 'Category Not Found' }} \ No newline at end of file diff --git a/app/templates/category/index.hbs b/app/templates/category/index.hbs new file mode 100644 index 00000000000..b7d88bde3e4 --- /dev/null +++ b/app/templates/category/index.hbs @@ -0,0 +1,59 @@ +{{ title category.category ' - Categories' }} + +
    + +

    All Crates

    +

    for category '{{ category.category }}'

    +
    + +
    + + +
    + Sort by + {{#rl-dropdown-container class="dropdown-container"}} + {{#rl-dropdown-toggle tagName="a" class="dropdown"}} + + {{ currentSortBy }} + + {{/rl-dropdown-toggle}} + + {{#rl-dropdown tagName="ul" class="dropdown" closeOnChildClick="a:link"}} +
  • + {{#link-to (query-params sort="alpha")}} + Alphabetical + {{/link-to}} +
  • +
  • + {{#link-to (query-params sort="downloads")}} + Downloads + {{/link-to}} +
  • + {{/rl-dropdown}} + {{/rl-dropdown-container}} +
    +
    + +
    + {{#each model as |crate|}} + {{crate-row crate=crate}} + {{/each}} +
    + + diff --git a/src/category.rs b/src/category.rs index c2f75c887c6..996d4be885a 100644 --- a/src/category.rs +++ b/src/category.rs @@ -1,12 +1,14 @@ use time::Timespec; use conduit::{Request, Response}; +use conduit_router::RequestParams; use pg::GenericConnection; use pg::rows::Row; use Model; use db::RequestTransaction; -use util::{RequestUtils, CargoResult}; +use util::{RequestUtils, CargoResult, ChainError}; +use util::errors::NotFound; #[derive(Clone)] pub struct Category { @@ -93,3 +95,15 @@ pub fn index(req: &mut Request) -> CargoResult { meta: Meta { total: total }, })) } + +/// Handles the `GET /categories/:category_id` route. +pub fn show(req: &mut Request) -> CargoResult { + let name = &req.params()["category_id"]; + let conn = try!(req.tx()); + let cat = try!(Category::find_by_category(&*conn, &name)); + let cat = try!(cat.chain_error(|| NotFound)); + + #[derive(RustcEncodable)] + struct R { category: EncodableCategory } + Ok(req.json(&R { category: cat.encodable() })) +} diff --git a/src/krate.rs b/src/krate.rs index 20519773838..a6d27c3676c 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -502,6 +502,19 @@ pub fn index(req: &mut Request) -> CargoResult { (format!("SELECT crates.* {} ORDER BY {} LIMIT $2 OFFSET $3", base, sort_sql), format!("SELECT COUNT(crates.*) {}", base)) }) + }).or_else(|| { + query.get("category").map(|cat| { + args.insert(0, cat); + let base = "FROM crates \ + INNER JOIN crates_categories \ + ON crates.id = crates_categories.crate_id \ + INNER JOIN categories \ + ON crates_categories.category_id = \ + categories.id \ + WHERE lower(categories.category) = lower($1)"; + (format!("SELECT crates.* {} ORDER BY {} LIMIT $2 OFFSET $3", base, sort_sql), + format!("SELECT COUNT(crates.*) {}", base)) + }) }).or_else(|| { query.get("user_id").and_then(|s| s.parse::().ok()).map(|user_id| { id = user_id; diff --git a/src/lib.rs b/src/lib.rs index 03385bed850..25fa0e7858c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -103,6 +103,7 @@ pub fn middleware(app: Arc) -> MiddlewareBuilder { api_router.get("/keywords", C(keyword::index)); api_router.get("/keywords/:keyword_id", C(keyword::show)); api_router.get("/categories", C(category::index)); + api_router.get("/categories/:category_id", C(category::show)); api_router.get("/users/:user_id", C(user::show)); let api_router = Arc::new(R404(api_router)); diff --git a/src/tests/category.rs b/src/tests/category.rs index fb8fb7743e0..5f5fc13f850 100644 --- a/src/tests/category.rs +++ b/src/tests/category.rs @@ -6,6 +6,8 @@ use cargo_registry::category::EncodableCategory; struct CategoryList { categories: Vec, meta: CategoryMeta } #[derive(RustcDecodable)] struct CategoryMeta { total: i32 } +#[derive(RustcDecodable)] +struct GoodCategory { category: EncodableCategory } #[test] fn index() { @@ -23,3 +25,16 @@ fn index() { assert_eq!(json.meta.total, 1); assert_eq!(json.categories[0].category, "foo"); } + +#[test] +fn show() { + let (_b, app, middle) = ::app(); + let mut req = ::req(app, Method::Get, "/api/v1/categories/foo"); + let response = t_resp!(middle.call(&mut req)); + assert_eq!(response.status.0, 404); + + ::mock_category(&mut req, "foo"); + let mut response = ok_resp!(middle.call(&mut req)); + let json: GoodCategory = ::json(&mut response); + assert_eq!(json.category.category, "foo"); +} From e46971d2dea2b23eed6ae4e74b3218b742677022 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 16 Nov 2016 16:45:30 -0500 Subject: [PATCH 05/34] Alphabetize mods I like when they match up with the filenames :) --- src/tests/all.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tests/all.rs b/src/tests/all.rs index f01eee904ef..7ae7124240b 100755 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -64,13 +64,13 @@ struct Error { detail: String } struct Bad { errors: Vec } mod category; +mod git; mod keyword; mod krate; -mod user; mod record; -mod git; -mod version; mod team; +mod user; +mod version; fn app() -> (record::Bomb, Arc, conduit_middleware::MiddlewareBuilder) { dotenv::dotenv().ok(); From 9c8ff2d9243da8d385ef59a7295c58fb1ed490fb Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 16 Nov 2016 17:34:29 -0500 Subject: [PATCH 06/34] Add the ability to add a category to a crate while publishing --- src/category.rs | 57 ++++++++++++++++++++++++++++++++++- src/krate.rs | 37 +++++++++++++++++++---- src/tests/all.rs | 8 +++-- src/tests/category.rs | 70 +++++++++++++++++++++++++++++++++++++++++-- src/tests/krate.rs | 7 +++-- src/tests/team.rs | 16 +++++++--- src/upload.rs | 42 ++++++++++++++++++++++++++ src/user/mod.rs | 2 +- 8 files changed, 220 insertions(+), 19 deletions(-) diff --git a/src/category.rs b/src/category.rs index 996d4be885a..ee70c24db59 100644 --- a/src/category.rs +++ b/src/category.rs @@ -1,3 +1,4 @@ +use std::collections::HashSet; use time::Timespec; use conduit::{Request, Response}; @@ -5,7 +6,7 @@ use conduit_router::RequestParams; use pg::GenericConnection; use pg::rows::Row; -use Model; +use {Model, Crate}; use db::RequestTransaction; use util::{RequestUtils, CargoResult, ChainError}; use util::errors::NotFound; @@ -35,6 +36,14 @@ impl Category { Ok(rows.iter().next().map(|r| Model::from_row(&r))) } + pub fn find_all_by_category(conn: &GenericConnection, names: &[String]) + -> CargoResult> { + let stmt = try!(conn.prepare("SELECT * FROM categories \ + WHERE category = ANY($1)")); + let rows = try!(stmt.query(&[&names])); + Ok(rows.iter().map(|r| Model::from_row(&r)).collect()) + } + pub fn encodable(self) -> EncodableCategory { let Category { id: _, crates_cnt, category, created_at } = self; EncodableCategory { @@ -44,6 +53,52 @@ impl Category { category: category, } } + + pub fn update_crate(conn: &GenericConnection, + krate: &Crate, + categories: &[String]) -> CargoResult<()> { + let old_categories = try!(krate.categories(conn)); + let old_categories_ids: HashSet<_> = old_categories.iter().map(|cat| { + cat.id + }).collect(); + // If a new category specified is not in the database, filter + // it out and don't add it. + let new_categories = try!( + Category::find_all_by_category(conn, categories) + ); + let new_categories_ids: HashSet<_> = new_categories.iter().map(|cat| { + cat.id + }).collect(); + + let to_rm: Vec<_> = old_categories_ids + .difference(&new_categories_ids) + .cloned() + .collect(); + let to_add: Vec<_> = new_categories_ids + .difference(&old_categories_ids) + .cloned() + .collect(); + + if to_rm.len() > 0 { + try!(conn.execute("DELETE FROM crates_categories \ + WHERE category_id = ANY($1) \ + AND crate_id = $2", + &[&to_rm, &krate.id])); + } + + if !to_add.is_empty() { + let insert: Vec<_> = to_add.into_iter().map(|id| { + format!("({}, {})", krate.id, id) + }).collect(); + let insert = insert.join(", "); + try!(conn.execute(&format!("INSERT INTO crates_categories \ + (crate_id, category_id) VALUES {}", + insert), + &[])); + } + + Ok(()) + } } impl Model for Category { diff --git a/src/krate.rs b/src/krate.rs index a6d27c3676c..4f2d55ce029 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -20,13 +20,14 @@ use semver; use time::{Timespec, Duration}; use url::Url; -use {Model, User, Keyword, Version}; +use {Model, User, Keyword, Version, Category}; use app::{App, RequestApp}; use db::RequestTransaction; use dependency::{Dependency, EncodableDependency}; use download::{VersionDownload, EncodableVersionDownload}; use git; use keyword::EncodableKeyword; +use category::EncodableCategory; use upload; use user::RequestUser; use owner::{EncodableOwner, Owner, Rights, OwnerKind, Team, rights}; @@ -59,6 +60,7 @@ pub struct EncodableCrate { pub updated_at: String, pub versions: Option>, pub keywords: Option>, + pub categories: Option>, pub created_at: String, pub downloads: i32, pub max_version: String, @@ -229,7 +231,8 @@ impl Crate { pub fn encodable(self, versions: Option>, - keywords: Option<&[Keyword]>) + keywords: Option<&[Keyword]>, + categories: Option<&[Category]>) -> EncodableCrate { let Crate { name, created_at, updated_at, downloads, max_version, description, @@ -241,6 +244,7 @@ impl Crate { None => Some(format!("/api/v1/crates/{}/versions", name)), }; let keyword_ids = keywords.map(|kws| kws.iter().map(|kw| kw.keyword.clone()).collect()); + let category_ids = categories.map(|cats| cats.iter().map(|cat| cat.category.clone()).collect()); EncodableCrate { id: name.clone(), name: name.clone(), @@ -249,6 +253,7 @@ impl Crate { downloads: downloads, versions: versions, keywords: keyword_ids, + categories: category_ids, max_version: max_version.to_string(), documentation: documentation, homepage: homepage, @@ -386,6 +391,16 @@ impl Crate { Ok(rows.iter().map(|r| Model::from_row(&r)).collect()) } + pub fn categories(&self, conn: &GenericConnection) -> CargoResult> { + let stmt = try!(conn.prepare("SELECT categories.* FROM categories \ + LEFT JOIN crates_categories \ + ON categories.id = \ + crates_categories.category_id \ + WHERE crates_categories.crate_id = $1")); + let rows = try!(stmt.query(&[&self.id])); + Ok(rows.iter().map(|r| Model::from_row(&r)).collect()) + } + /// Returns (dependency, dependent crate name) pub fn reverse_dependencies(&self, conn: &GenericConnection, @@ -567,7 +582,7 @@ pub fn index(req: &mut Request) -> CargoResult { let mut crates = Vec::new(); for row in try!(stmt.query(&args)).iter() { let krate: Crate = Model::from_row(&row); - crates.push(krate.encodable(None, None)); + crates.push(krate.encodable(None, None, None)); } // Query for the total count of crates @@ -602,7 +617,7 @@ pub fn summary(req: &mut Request) -> CargoResult { let rows = try!(stmt.query(&[])); Ok(rows.iter().map(|r| { let krate: Crate = Model::from_row(&r); - krate.encodable(None, None) + krate.encodable(None, None, None) }).collect::>()) }; let new_crates = try!(tx.prepare("SELECT * FROM crates \ @@ -639,19 +654,22 @@ pub fn show(req: &mut Request) -> CargoResult { let versions = try!(krate.versions(conn)); let ids = versions.iter().map(|v| v.id).collect(); let kws = try!(krate.keywords(conn)); + let cats = try!(krate.categories(conn)); #[derive(RustcEncodable)] struct R { krate: EncodableCrate, versions: Vec, keywords: Vec, + categories: Vec, } Ok(req.json(&R { - krate: krate.clone().encodable(Some(ids), Some(&kws)), + krate: krate.clone().encodable(Some(ids), Some(&kws), Some(&cats)), versions: versions.into_iter().map(|v| { v.encodable(&krate.name) }).collect(), keywords: kws.into_iter().map(|k| k.encodable()).collect(), + categories: cats.into_iter().map(|k| k.encodable()).collect(), })) } @@ -669,6 +687,10 @@ pub fn new(req: &mut Request) -> CargoResult { .unwrap_or(&[]); let keywords = keywords.iter().map(|k| k[..].to_string()).collect::>(); + let categories = new_crate.categories.as_ref().map(|s| &s[..]) + .unwrap_or(&[]); + let categories = categories.iter().map(|k| k[..].to_string()).collect::>(); + // Persist the new crate, if it doesn't already exist let mut krate = try!(Crate::find_or_insert(try!(req.tx()), name, user.id, &new_crate.description, @@ -713,6 +735,9 @@ pub fn new(req: &mut Request) -> CargoResult { // Update all keywords for this crate try!(Keyword::update_crate(try!(req.tx()), &krate, &keywords)); + // Update all categories for this crate + try!(Category::update_crate(try!(req.tx()), &krate, &categories)); + // Upload the crate to S3 let mut handle = req.app().handle(); let path = krate.s3_path(&vers.to_string()); @@ -771,7 +796,7 @@ pub fn new(req: &mut Request) -> CargoResult { #[derive(RustcEncodable)] struct R { krate: EncodableCrate } - Ok(req.json(&R { krate: krate.encodable(None, None) })) + Ok(req.json(&R { krate: krate.encodable(None, None, None) })) } fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> { diff --git a/src/tests/all.rs b/src/tests/all.rs index 7ae7124240b..ca20ffa2acd 100755 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -272,20 +272,21 @@ fn new_req(app: Arc, krate: &str, version: &str) -> MockRequest { fn new_req_full(app: Arc, krate: Crate, version: &str, deps: Vec) -> MockRequest { let mut req = ::req(app, Method::Put, "/api/v1/crates/new"); - req.with_body(&new_req_body(krate, version, deps, Vec::new())); + req.with_body(&new_req_body(krate, version, deps, Vec::new(), Vec::new())); return req; } fn new_req_with_keywords(app: Arc, krate: Crate, version: &str, kws: Vec) -> MockRequest { let mut req = ::req(app, Method::Put, "/api/v1/crates/new"); - req.with_body(&new_req_body(krate, version, Vec::new(), kws)); + req.with_body(&new_req_body(krate, version, Vec::new(), kws, Vec::new())); return req; } fn new_req_body(krate: Crate, version: &str, deps: Vec, - kws: Vec) -> Vec { + kws: Vec, cats: Vec) -> Vec { let kws = kws.into_iter().map(u::Keyword).collect(); + let cats = cats.into_iter().map(u::Category).collect(); new_crate_to_body(&u::NewCrate { name: u::CrateName(krate.name), vers: u::CrateVersion(semver::Version::parse(version).unwrap()), @@ -297,6 +298,7 @@ fn new_req_body(krate: Crate, version: &str, deps: Vec, documentation: krate.documentation, readme: krate.readme, keywords: Some(u::KeywordList(kws)), + categories: Some(u::CategoryList(cats)), license: Some("MIT".to_string()), license_file: None, repository: krate.repository, diff --git a/src/tests/category.rs b/src/tests/category.rs index 5f5fc13f850..637fa3a401e 100644 --- a/src/tests/category.rs +++ b/src/tests/category.rs @@ -1,6 +1,9 @@ -use conduit::{Handler, Method}; +use postgres::GenericConnection; +use conduit::{Handler, Request, Method}; +use conduit_test::MockRequest; -use cargo_registry::category::EncodableCategory; +use cargo_registry::db::RequestTransaction; +use cargo_registry::category::{Category, EncodableCategory}; #[derive(RustcDecodable)] struct CategoryList { categories: Vec, meta: CategoryMeta } @@ -38,3 +41,66 @@ fn show() { let json: GoodCategory = ::json(&mut response); assert_eq!(json.category.category, "foo"); } + +fn tx(req: &Request) -> &GenericConnection { req.tx().unwrap() } + +#[test] +fn update_crate() { + let (_b, app, middle) = ::app(); + let mut req = ::req(app, Method::Get, "/api/v1/categories/foo"); + let cnt = |req: &mut MockRequest, cat: &str| { + req.with_path(&format!("/api/v1/categories/{}", cat)); + let mut response = ok_resp!(middle.call(req)); + ::json::(&mut response).category.crates_cnt as usize + }; + ::mock_user(&mut req, ::user("foo")); + let (krate, _) = ::mock_crate(&mut req, ::krate("foo")); + ::mock_category(&mut req, "cat1"); + ::mock_category(&mut req, "cat2"); + + // Updating with no categories has no effect + Category::update_crate(tx(&req), &krate, &[]).unwrap(); + assert_eq!(cnt(&mut req, "cat1"), 0); + assert_eq!(cnt(&mut req, "cat2"), 0); + + // Happy path adding one category + Category::update_crate(tx(&req), &krate, &["cat1".to_string()]).unwrap(); + assert_eq!(cnt(&mut req, "cat1"), 1); + assert_eq!(cnt(&mut req, "cat2"), 0); + + // Replacing one category with another + Category::update_crate(tx(&req), &krate, &["cat2".to_string()]).unwrap(); + assert_eq!(cnt(&mut req, "cat1"), 0); + assert_eq!(cnt(&mut req, "cat2"), 1); + + // Removing one category + Category::update_crate(tx(&req), &krate, &[]).unwrap(); + assert_eq!(cnt(&mut req, "cat1"), 0); + assert_eq!(cnt(&mut req, "cat2"), 0); + + // Adding 2 categories + Category::update_crate(tx(&req), &krate, &["cat1".to_string(), + "cat2".to_string()]).unwrap(); + assert_eq!(cnt(&mut req, "cat1"), 1); + assert_eq!(cnt(&mut req, "cat2"), 1); + + // Removing all categories + Category::update_crate(tx(&req), &krate, &[]).unwrap(); + assert_eq!(cnt(&mut req, "cat1"), 0); + assert_eq!(cnt(&mut req, "cat2"), 0); + + // Attempting to add one valid category and one invalid category + Category::update_crate(tx(&req), &krate, &["cat1".to_string(), + "catnope".to_string()]).unwrap(); + + assert_eq!(cnt(&mut req, "cat1"), 1); + assert_eq!(cnt(&mut req, "cat2"), 0); + + // Does not add the invalid category to the category list + // (unlike the behavior of keywords) + req.with_path("/api/v1/categories"); + let mut response = ok_resp!(middle.call(&mut req)); + let json: CategoryList = ::json(&mut response); + assert_eq!(json.categories.len(), 2); + assert_eq!(json.meta.total, 2); +} diff --git a/src/tests/krate.rs b/src/tests/krate.rs index e9ff94d06e3..3561039fd67 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -48,6 +48,7 @@ fn new_crate(name: &str) -> u::NewCrate { documentation: None, readme: None, keywords: None, + categories: None, license: Some("MIT".to_string()), license_file: None, repository: None, @@ -190,7 +191,7 @@ fn exact_match_on_queries_with_sort() { krate4.description = Some("other const".to_string()); krate4.downloads = 999999; let (k4, _) = ::mock_crate(&mut req, krate4.clone()); - + { let req2: &mut Request = &mut req; let tx = req2.tx().unwrap(); @@ -461,7 +462,9 @@ fn new_crate_owner() { assert_eq!(::json::(&mut response).crates.len(), 1); // And upload a new crate as the first user - let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new()); + let body = ::new_req_body( + ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() + ); req.mut_extensions().insert(u2); let mut response = ok_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_method(Method::Put) diff --git a/src/tests/team.rs b/src/tests/team.rs index 3e02975b0db..132b5d376d4 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -134,7 +134,9 @@ fn remove_team_as_named_owner() { .with_body(body.as_bytes()))); ::mock_user(&mut req, mock_user_on_only_x()); - let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new()); + let body = ::new_req_body( + ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() + ); let json = bad_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); @@ -164,7 +166,9 @@ fn remove_team_as_team_owner() { assert!(json.errors[0].detail.contains("don't have permission"), "{:?}", json.errors); - let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new()); + let body = ::new_req_body( + ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() + ); ok_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); @@ -185,7 +189,9 @@ fn publish_not_owned() { .with_body(body.as_bytes()))); ::mock_user(&mut req, mock_user_on_only_x()); - let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new()); + let body = ::new_req_body( + ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() + ); let json = bad_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); @@ -207,7 +213,9 @@ fn publish_owned() { .with_body(body.as_bytes()))); ::mock_user(&mut req, mock_user_on_only_x()); - let body = ::new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new()); + let body = ::new_req_body( + ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() + ); ok_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); diff --git a/src/upload.rs b/src/upload.rs index 1759d5da11b..72a8ee32ba7 100644 --- a/src/upload.rs +++ b/src/upload.rs @@ -20,6 +20,7 @@ pub struct NewCrate { pub documentation: Option, pub readme: Option, pub keywords: Option, + pub categories: Option, pub license: Option, pub license_file: Option, pub repository: Option, @@ -31,6 +32,8 @@ pub struct CrateVersion(pub semver::Version); pub struct CrateVersionReq(pub semver::VersionReq); pub struct KeywordList(pub Vec); pub struct Keyword(pub String); +pub struct CategoryList(pub Vec); +pub struct Category(pub String); pub struct Feature(pub String); #[derive(RustcDecodable, RustcEncodable)] @@ -64,6 +67,12 @@ impl Decodable for Keyword { } } +impl Decodable for Category { + fn decode(d: &mut D) -> Result { + d.read_str().map(Category) + } +} + impl Decodable for Feature { fn decode(d: &mut D) -> Result { let s = try!(d.read_str()); @@ -110,6 +119,16 @@ impl Decodable for KeywordList { } } +impl Decodable for CategoryList { + fn decode(d: &mut D) -> Result { + let inner: Vec = try!(Decodable::decode(d)); + if inner.len() > 5 { + return Err(d.error("a maximum of 5 categories per crate are allowed")) + } + Ok(CategoryList(inner)) + } +} + impl Decodable for DependencyKind { fn decode(d: &mut D) -> Result { let s: String = try!(Decodable::decode(d)); @@ -135,6 +154,12 @@ impl Encodable for Keyword { } } +impl Encodable for Category { + fn encode(&self, d: &mut E) -> Result<(), E::Error> { + d.emit_str(self) + } +} + impl Encodable for Feature { fn encode(&self, d: &mut E) -> Result<(), E::Error> { d.emit_str(self) @@ -160,6 +185,13 @@ impl Encodable for KeywordList { } } +impl Encodable for CategoryList { + fn encode(&self, d: &mut E) -> Result<(), E::Error> { + let CategoryList(ref inner) = *self; + inner.encode(d) + } +} + impl Encodable for DependencyKind { fn encode(&self, d: &mut E) -> Result<(), E::Error> { match *self { @@ -180,6 +212,11 @@ impl Deref for Keyword { fn deref(&self) -> &str { &self.0 } } +impl Deref for Category { + type Target = str; + fn deref(&self) -> &str { &self.0 } +} + impl Deref for Feature { type Target = str; fn deref(&self) -> &str { &self.0 } @@ -203,3 +240,8 @@ impl Deref for KeywordList { type Target = [Keyword]; fn deref(&self) -> &[Keyword] { &self.0 } } + +impl Deref for CategoryList { + type Target = [Category]; + fn deref(&self) -> &[Category] { &self.0 } +} diff --git a/src/user/mod.rs b/src/user/mod.rs index 66d20efa089..af68c66a8dc 100644 --- a/src/user/mod.rs +++ b/src/user/mod.rs @@ -336,7 +336,7 @@ pub fn updates(req: &mut Request) -> CargoResult { } // Encode everything! - let crates = crates.into_iter().map(|c| c.encodable(None, None)).collect(); + let crates = crates.into_iter().map(|c| c.encodable(None, None, None)).collect(); let versions = versions.into_iter().map(|v| { let id = v.crate_id; v.encodable(&map[&id]) From 5e8d7be8cb76d64774bc412a7c58d3af5a453e20 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 16 Nov 2016 18:53:58 -0500 Subject: [PATCH 07/34] Extract a method for encoding a crate without all its metadata I draw the line at 3 None arguments. --- src/krate.rs | 10 +++++++--- src/user/mod.rs | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 4f2d55ce029..dbbe0245be3 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -229,6 +229,10 @@ impl Crate { parts.next().is_none() } + pub fn minimal_encodable(self) -> EncodableCrate { + self.encodable(None, None, None) + } + pub fn encodable(self, versions: Option>, keywords: Option<&[Keyword]>, @@ -582,7 +586,7 @@ pub fn index(req: &mut Request) -> CargoResult { let mut crates = Vec::new(); for row in try!(stmt.query(&args)).iter() { let krate: Crate = Model::from_row(&row); - crates.push(krate.encodable(None, None, None)); + crates.push(krate.minimal_encodable()); } // Query for the total count of crates @@ -617,7 +621,7 @@ pub fn summary(req: &mut Request) -> CargoResult { let rows = try!(stmt.query(&[])); Ok(rows.iter().map(|r| { let krate: Crate = Model::from_row(&r); - krate.encodable(None, None, None) + krate.minimal_encodable() }).collect::>()) }; let new_crates = try!(tx.prepare("SELECT * FROM crates \ @@ -796,7 +800,7 @@ pub fn new(req: &mut Request) -> CargoResult { #[derive(RustcEncodable)] struct R { krate: EncodableCrate } - Ok(req.json(&R { krate: krate.encodable(None, None, None) })) + Ok(req.json(&R { krate: krate.minimal_encodable() })) } fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> { diff --git a/src/user/mod.rs b/src/user/mod.rs index af68c66a8dc..1b3ea1853d4 100644 --- a/src/user/mod.rs +++ b/src/user/mod.rs @@ -336,7 +336,7 @@ pub fn updates(req: &mut Request) -> CargoResult { } // Encode everything! - let crates = crates.into_iter().map(|c| c.encodable(None, None, None)).collect(); + let crates = crates.into_iter().map(|c| c.minimal_encodable()).collect(); let versions = versions.into_iter().map(|v| { let id = v.crate_id; v.encodable(&map[&id]) From 7cb99cf8e65a564d44179b8428341a1759137090 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 16 Nov 2016 19:02:09 -0500 Subject: [PATCH 08/34] Extract a test helper method for uploading foo 2.0.0 --- src/tests/all.rs | 4 ++++ src/tests/krate.rs | 4 +--- src/tests/team.rs | 16 ++++------------ 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/tests/all.rs b/src/tests/all.rs index ca20ffa2acd..f78e65e722d 100755 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -283,6 +283,10 @@ fn new_req_with_keywords(app: Arc, krate: Crate, version: &str, return req; } +fn new_req_body_foo_version_2() -> Vec { + new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new()) +} + fn new_req_body(krate: Crate, version: &str, deps: Vec, kws: Vec, cats: Vec) -> Vec { let kws = kws.into_iter().map(u::Keyword).collect(); diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 3561039fd67..68dca89c738 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -462,9 +462,7 @@ fn new_crate_owner() { assert_eq!(::json::(&mut response).crates.len(), 1); // And upload a new crate as the first user - let body = ::new_req_body( - ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() - ); + let body = ::new_req_body_foo_version_2(); req.mut_extensions().insert(u2); let mut response = ok_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_method(Method::Put) diff --git a/src/tests/team.rs b/src/tests/team.rs index 132b5d376d4..cc5429352c8 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -134,9 +134,7 @@ fn remove_team_as_named_owner() { .with_body(body.as_bytes()))); ::mock_user(&mut req, mock_user_on_only_x()); - let body = ::new_req_body( - ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() - ); + let body = ::new_req_body_foo_version_2(); let json = bad_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); @@ -166,9 +164,7 @@ fn remove_team_as_team_owner() { assert!(json.errors[0].detail.contains("don't have permission"), "{:?}", json.errors); - let body = ::new_req_body( - ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() - ); + let body = ::new_req_body_foo_version_2(); ok_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); @@ -189,9 +185,7 @@ fn publish_not_owned() { .with_body(body.as_bytes()))); ::mock_user(&mut req, mock_user_on_only_x()); - let body = ::new_req_body( - ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() - ); + let body = ::new_req_body_foo_version_2(); let json = bad_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); @@ -213,9 +207,7 @@ fn publish_owned() { .with_body(body.as_bytes()))); ::mock_user(&mut req, mock_user_on_only_x()); - let body = ::new_req_body( - ::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new() - ); + let body = ::new_req_body_foo_version_2(); ok_resp!(middle.call(req.with_path("/api/v1/crates/new") .with_body(&body) .with_method(Method::Put))); From c8442ed6b1958c2ed722fca81760215cad901415 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Wed, 16 Nov 2016 19:16:51 -0500 Subject: [PATCH 09/34] Show categories on a crate's page --- app/controllers/crate/version.js | 2 ++ app/models/crate.js | 1 + app/templates/crate/version.hbs | 13 +++++++++++++ 3 files changed, 16 insertions(+) diff --git a/app/controllers/crate/version.js b/app/controllers/crate/version.js index ad0c5836a88..99216589ed0 100644 --- a/app/controllers/crate/version.js +++ b/app/controllers/crate/version.js @@ -19,6 +19,7 @@ export default Ember.Controller.extend({ currentVersion: computed.alias('model'), requestedVersion: null, keywords: computed.alias('crate.keywords'), + categories: computed.alias('crate.categories'), sortedVersions: computed.readOnly('crate.versions'), @@ -49,6 +50,7 @@ export default Ember.Controller.extend({ }), anyKeywords: computed.gt('keywords.length', 0), + anyCategories: computed.gt('categories.length', 0), currentDependencies: computed('currentVersion.dependencies', function() { var deps = this.get('currentVersion.dependencies'); diff --git a/app/models/crate.js b/app/models/crate.js index 73061e89a12..9065b323f7e 100644 --- a/app/models/crate.js +++ b/app/models/crate.js @@ -20,6 +20,7 @@ export default DS.Model.extend({ owners: DS.hasMany('users', { async: true }), version_downloads: DS.hasMany('version-download', { async: true }), keywords: DS.hasMany('keywords', { async: true }), + categories: DS.hasMany('categories', { async: true }), reverse_dependencies: DS.hasMany('dependency', { async: true }), follow() { diff --git a/app/templates/crate/version.hbs b/app/templates/crate/version.hbs index 42356ec38ec..fd0543e2c3d 100644 --- a/app/templates/crate/version.hbs +++ b/app/templates/crate/version.hbs @@ -104,6 +104,19 @@ {{/if}} {{/unless}} + {{#unless crate.categories.isPending}} + {{#if anyCategories}} +
    +

    Categories

    +
      + {{#each categories as |category|}} +
    • {{link-to category.id 'category' category}}
    • + {{/each}} +
    +
    + {{/if}} + {{/unless}} +

    Owners

      From 54c374b7b75d9f9ff4c0e1c6b6bd93318b19d025 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 17 Nov 2016 09:47:25 -0500 Subject: [PATCH 10/34] Prefer as_str over &[..] --- src/category.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/category.rs b/src/category.rs index ee70c24db59..adf96869e76 100644 --- a/src/category.rs +++ b/src/category.rs @@ -118,7 +118,7 @@ pub fn index(req: &mut Request) -> CargoResult { let conn = try!(req.tx()); let (offset, limit) = try!(req.pagination(10, 100)); let query = req.query(); - let sort = query.get("sort").map(|s| &s[..]).unwrap_or("alpha"); + let sort = query.get("sort").map_or("alpha", String::as_str); let sort_sql = match sort { "crates" => "ORDER BY crates_cnt DESC", _ => "ORDER BY category ASC", From fcd7b28ddfacd01392a42496254c584864072680 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 17 Nov 2016 09:55:52 -0500 Subject: [PATCH 11/34] Prefer type annotations over turbofish --- src/krate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/krate.rs b/src/krate.rs index dbbe0245be3..8a52bf5a47a 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -693,7 +693,7 @@ pub fn new(req: &mut Request) -> CargoResult { let categories = new_crate.categories.as_ref().map(|s| &s[..]) .unwrap_or(&[]); - let categories = categories.iter().map(|k| k[..].to_string()).collect::>(); + let categories: Vec<_> = categories.iter().map(|k| k[..].to_string()).collect(); // Persist the new crate, if it doesn't already exist let mut krate = try!(Crate::find_or_insert(try!(req.tx()), name, user.id, From 5a8ad82f5dd2e87661831ab483fdc4ad7324ec8b Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Thu, 17 Nov 2016 09:59:50 -0500 Subject: [PATCH 12/34] Prefer not is empty over len greater than zero --- src/category.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/category.rs b/src/category.rs index adf96869e76..9ad8fdd3c97 100644 --- a/src/category.rs +++ b/src/category.rs @@ -79,7 +79,7 @@ impl Category { .cloned() .collect(); - if to_rm.len() > 0 { + if !to_rm.is_empty() { try!(conn.execute("DELETE FROM crates_categories \ WHERE category_id = ANY($1) \ AND crate_id = $2", From 6364c73263c787b1bfa2cde73be3c3388ee2c21d Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 21 Nov 2016 13:58:52 -0500 Subject: [PATCH 13/34] Move chain_error NotFound into find_by_category --- src/category.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/category.rs b/src/category.rs index 9ad8fdd3c97..07f6575c1bc 100644 --- a/src/category.rs +++ b/src/category.rs @@ -29,11 +29,13 @@ pub struct EncodableCategory { impl Category { pub fn find_by_category(conn: &GenericConnection, name: &str) - -> CargoResult> { + -> CargoResult { let stmt = try!(conn.prepare("SELECT * FROM categories \ WHERE category = $1")); let rows = try!(stmt.query(&[&name])); - Ok(rows.iter().next().map(|r| Model::from_row(&r))) + rows.iter().next() + .chain_error(|| NotFound) + .map(|row| Model::from_row(&row)) } pub fn find_all_by_category(conn: &GenericConnection, names: &[String]) @@ -156,7 +158,6 @@ pub fn show(req: &mut Request) -> CargoResult { let name = &req.params()["category_id"]; let conn = try!(req.tx()); let cat = try!(Category::find_by_category(&*conn, &name)); - let cat = try!(cat.chain_error(|| NotFound)); #[derive(RustcEncodable)] struct R { category: EncodableCategory } From 29f8e2e41a2dfad63ce4b6e40bd371eb6bf9e647 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 21 Nov 2016 15:15:07 -0500 Subject: [PATCH 14/34] Use a pretty slug for categories in URLs Categories are now specified by slug in Cargo.toml. This will allow crates.io to change the display text of a category but still have crates in those categories. Characters allowed in slugs are from RFC 3986, those that are valid in path segments (pchar) https://tools.ietf.org/html/rfc3986#page-22 --- app/models/category.js | 1 + app/templates/categories.hbs | 2 +- app/templates/crate/version.hbs | 2 +- src/bin/migrate.rs | 1 + src/bin/sync-categories.rs | 30 +++++++++++++--------- src/categories.txt | 5 ++-- src/category.rs | 32 +++++++++++++++--------- src/krate.rs | 2 +- src/tests/all.rs | 8 +++--- src/tests/category.rs | 44 +++++++++++++++++++++------------ 10 files changed, 78 insertions(+), 49 deletions(-) diff --git a/app/models/category.js b/app/models/category.js index 6a950224a9c..24efc91cd18 100644 --- a/app/models/category.js +++ b/app/models/category.js @@ -2,6 +2,7 @@ import DS from 'ember-data'; export default DS.Model.extend({ category: DS.attr('string'), + slug: DS.attr('string'), created_at: DS.attr('date'), crates_cnt: DS.attr('number'), diff --git a/app/templates/categories.hbs b/app/templates/categories.hbs index 71cd2d8d21f..8239da8a4c1 100644 --- a/app/templates/categories.hbs +++ b/app/templates/categories.hbs @@ -43,7 +43,7 @@ {{#each model as |category|}}
      - {{link-to category.id "category" category}} + {{link-to category.category "category" category.slug}} {{ format-num category.crates_cnt }} crates diff --git a/app/templates/crate/version.hbs b/app/templates/crate/version.hbs index fd0543e2c3d..419f52d5a80 100644 --- a/app/templates/crate/version.hbs +++ b/app/templates/crate/version.hbs @@ -110,7 +110,7 @@

      Categories

        {{#each categories as |category|}} -
      • {{link-to category.id 'category' category}}
      • +
      • {{link-to category.category 'category' category.slug}}
      • {{/each}}
      diff --git a/src/bin/migrate.rs b/src/bin/migrate.rs index 0b6d76121e8..353756311f5 100644 --- a/src/bin/migrate.rs +++ b/src/bin/migrate.rs @@ -767,6 +767,7 @@ fn migrations() -> Vec { Migration::add_table(20161115110541, "categories", " \ id SERIAL PRIMARY KEY, \ category VARCHAR NOT NULL UNIQUE, \ + slug VARCHAR NOT NULL UNIQUE, \ crates_cnt INTEGER NOT NULL DEFAULT 0, \ created_at TIMESTAMP NOT NULL DEFAULT current_timestamp"), Migration::add_table(20161115111828, "crates_categories", " \ diff --git a/src/bin/sync-categories.rs b/src/bin/sync-categories.rs index 86b68daea9a..9517bd00d72 100644 --- a/src/bin/sync-categories.rs +++ b/src/bin/sync-categories.rs @@ -23,23 +23,29 @@ fn main() { fn sync(tx: &postgres::transaction::Transaction) -> postgres::Result<()> { let categories = include_str!("../categories.txt"); - let categories: Vec<_> = categories.lines().collect(); - let insert = categories.iter() - .map(|c| format!("('{}')", c)) - .collect::>() - .join(","); - let in_clause = categories.iter() - .map(|c| format!("'{}'", c)) - .collect::>() - .join(","); + + let slug_categories: Vec<_> = categories.lines().map(|c| { + let mut parts = c.split(' '); + let slug = parts.next().expect("No slug found!"); + let name = parts.collect::>().join(" "); + (slug, name) + }).collect(); + + let insert = slug_categories.iter().map(|&(ref slug, ref name)| { + format!("(LOWER('{}'), '{}')", slug, name) + }).collect::>().join(","); + + let in_clause = slug_categories.iter().map(|&(slug, _)| { + format!("LOWER('{}')", slug) + }).collect::>().join(","); try!(tx.batch_execute( &format!(" \ - INSERT INTO categories (category) \ + INSERT INTO categories (slug, category) \ VALUES {} \ - ON CONFLICT (category) DO NOTHING; \ + ON CONFLICT (slug) DO UPDATE SET category = EXCLUDED.category; \ DELETE FROM categories \ - WHERE category NOT IN ({});", + WHERE slug NOT IN ({});", insert, in_clause )[..] diff --git a/src/categories.txt b/src/categories.txt index 2447433817a..1133607f2db 100644 --- a/src/categories.txt +++ b/src/categories.txt @@ -1,2 +1,3 @@ -Test -Attempt +test Test +attempt Attempt +node.js-ruby-ffi Node.js/Ruby FFI diff --git a/src/category.rs b/src/category.rs index 07f6575c1bc..d699c9f924e 100644 --- a/src/category.rs +++ b/src/category.rs @@ -15,6 +15,7 @@ use util::errors::NotFound; pub struct Category { pub id: i32, pub category: String, + pub slug: String, pub created_at: Timespec, pub crates_cnt: i32, } @@ -23,6 +24,7 @@ pub struct Category { pub struct EncodableCategory { pub id: String, pub category: String, + pub slug: String, pub created_at: String, pub crates_cnt: i32, } @@ -38,18 +40,21 @@ impl Category { .map(|row| Model::from_row(&row)) } - pub fn find_all_by_category(conn: &GenericConnection, names: &[String]) - -> CargoResult> { + pub fn find_by_slug(conn: &GenericConnection, slug: &str) + -> CargoResult { let stmt = try!(conn.prepare("SELECT * FROM categories \ - WHERE category = ANY($1)")); - let rows = try!(stmt.query(&[&names])); - Ok(rows.iter().map(|r| Model::from_row(&r)).collect()) + WHERE slug = LOWER($1)")); + let rows = try!(stmt.query(&[&slug])); + rows.iter().next() + .chain_error(|| NotFound) + .map(|row| Model::from_row(&row)) } pub fn encodable(self) -> EncodableCategory { - let Category { id: _, crates_cnt, category, created_at } = self; + let Category { id: _, crates_cnt, category, slug, created_at } = self; EncodableCategory { - id: category.clone(), + id: slug.clone(), + slug: slug.clone(), created_at: ::encode_time(created_at), crates_cnt: crates_cnt, category: category, @@ -63,11 +68,13 @@ impl Category { let old_categories_ids: HashSet<_> = old_categories.iter().map(|cat| { cat.id }).collect(); + // If a new category specified is not in the database, filter // it out and don't add it. - let new_categories = try!( - Category::find_all_by_category(conn, categories) - ); + let new_categories: Vec = categories.iter().flat_map(|c| { + Category::find_by_slug(conn, &c).ok() + }).collect(); + let new_categories_ids: HashSet<_> = new_categories.iter().map(|cat| { cat.id }).collect(); @@ -110,6 +117,7 @@ impl Model for Category { created_at: row.get("created_at"), crates_cnt: row.get("crates_cnt"), category: row.get("category"), + slug: row.get("slug"), } } fn table_name(_: Option) -> &'static str { "categories" } @@ -155,9 +163,9 @@ pub fn index(req: &mut Request) -> CargoResult { /// Handles the `GET /categories/:category_id` route. pub fn show(req: &mut Request) -> CargoResult { - let name = &req.params()["category_id"]; + let slug = &req.params()["category_id"]; let conn = try!(req.tx()); - let cat = try!(Category::find_by_category(&*conn, &name)); + let cat = try!(Category::find_by_slug(&*conn, &slug)); #[derive(RustcEncodable)] struct R { category: EncodableCategory } diff --git a/src/krate.rs b/src/krate.rs index 8a52bf5a47a..5a67d5c22b6 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -248,7 +248,7 @@ impl Crate { None => Some(format!("/api/v1/crates/{}/versions", name)), }; let keyword_ids = keywords.map(|kws| kws.iter().map(|kw| kw.keyword.clone()).collect()); - let category_ids = categories.map(|cats| cats.iter().map(|cat| cat.category.clone()).collect()); + let category_ids = categories.map(|cats| cats.iter().map(|cat| cat.slug.clone()).collect()); EncodableCrate { id: name.clone(), name: name.clone(), diff --git a/src/tests/all.rs b/src/tests/all.rs index f78e65e722d..b175b891d27 100755 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -251,13 +251,13 @@ fn mock_keyword(req: &mut Request, name: &str) -> Keyword { Keyword::find_or_insert(req.tx().unwrap(), name).unwrap() } -fn mock_category(req: &mut Request, name: &str) -> Category { +fn mock_category(req: &mut Request, name: &str, slug: &str) -> Category { let conn = req.tx().unwrap(); let stmt = conn.prepare(" \ - INSERT INTO categories (category) \ - VALUES ($1) \ + INSERT INTO categories (category, slug) \ + VALUES ($1, $2) \ RETURNING *").unwrap(); - let rows = stmt.query(&[&name]).unwrap(); + let rows = stmt.query(&[&name, &slug]).unwrap(); Model::from_row(&rows.iter().next().unwrap()) } diff --git a/src/tests/category.rs b/src/tests/category.rs index 637fa3a401e..a9f088ba03e 100644 --- a/src/tests/category.rs +++ b/src/tests/category.rs @@ -21,7 +21,7 @@ fn index() { assert_eq!(json.categories.len(), 0); assert_eq!(json.meta.total, 0); - ::mock_category(&mut req, "foo"); + ::mock_category(&mut req, "foo", "foo"); let mut response = ok_resp!(middle.call(&mut req)); let json: CategoryList = ::json(&mut response); assert_eq!(json.categories.len(), 1); @@ -32,14 +32,15 @@ fn index() { #[test] fn show() { let (_b, app, middle) = ::app(); - let mut req = ::req(app, Method::Get, "/api/v1/categories/foo"); + let mut req = ::req(app, Method::Get, "/api/v1/categories/foo-bar"); let response = t_resp!(middle.call(&mut req)); assert_eq!(response.status.0, 404); - ::mock_category(&mut req, "foo"); + ::mock_category(&mut req, "Foo Bar", "foo-bar"); let mut response = ok_resp!(middle.call(&mut req)); let json: GoodCategory = ::json(&mut response); - assert_eq!(json.category.category, "foo"); + assert_eq!(json.category.category, "Foo Bar"); + assert_eq!(json.category.slug, "foo-bar"); } fn tx(req: &Request) -> &GenericConnection { req.tx().unwrap() } @@ -55,46 +56,49 @@ fn update_crate() { }; ::mock_user(&mut req, ::user("foo")); let (krate, _) = ::mock_crate(&mut req, ::krate("foo")); - ::mock_category(&mut req, "cat1"); - ::mock_category(&mut req, "cat2"); + ::mock_category(&mut req, "cat1", "cat1"); + ::mock_category(&mut req, "Category 2", "category-2"); // Updating with no categories has no effect Category::update_crate(tx(&req), &krate, &[]).unwrap(); assert_eq!(cnt(&mut req, "cat1"), 0); - assert_eq!(cnt(&mut req, "cat2"), 0); + assert_eq!(cnt(&mut req, "category-2"), 0); // Happy path adding one category Category::update_crate(tx(&req), &krate, &["cat1".to_string()]).unwrap(); assert_eq!(cnt(&mut req, "cat1"), 1); - assert_eq!(cnt(&mut req, "cat2"), 0); + assert_eq!(cnt(&mut req, "category-2"), 0); // Replacing one category with another - Category::update_crate(tx(&req), &krate, &["cat2".to_string()]).unwrap(); + Category::update_crate( + tx(&req), &krate, &["category-2".to_string()] + ).unwrap(); assert_eq!(cnt(&mut req, "cat1"), 0); - assert_eq!(cnt(&mut req, "cat2"), 1); + assert_eq!(cnt(&mut req, "category-2"), 1); // Removing one category Category::update_crate(tx(&req), &krate, &[]).unwrap(); assert_eq!(cnt(&mut req, "cat1"), 0); - assert_eq!(cnt(&mut req, "cat2"), 0); + assert_eq!(cnt(&mut req, "category-2"), 0); // Adding 2 categories - Category::update_crate(tx(&req), &krate, &["cat1".to_string(), - "cat2".to_string()]).unwrap(); + Category::update_crate( + tx(&req), &krate, &["cat1".to_string(), + "category-2".to_string()]).unwrap(); assert_eq!(cnt(&mut req, "cat1"), 1); - assert_eq!(cnt(&mut req, "cat2"), 1); + assert_eq!(cnt(&mut req, "category-2"), 1); // Removing all categories Category::update_crate(tx(&req), &krate, &[]).unwrap(); assert_eq!(cnt(&mut req, "cat1"), 0); - assert_eq!(cnt(&mut req, "cat2"), 0); + assert_eq!(cnt(&mut req, "category-2"), 0); // Attempting to add one valid category and one invalid category Category::update_crate(tx(&req), &krate, &["cat1".to_string(), "catnope".to_string()]).unwrap(); assert_eq!(cnt(&mut req, "cat1"), 1); - assert_eq!(cnt(&mut req, "cat2"), 0); + assert_eq!(cnt(&mut req, "category-2"), 0); // Does not add the invalid category to the category list // (unlike the behavior of keywords) @@ -103,4 +107,12 @@ fn update_crate() { let json: CategoryList = ::json(&mut response); assert_eq!(json.categories.len(), 2); assert_eq!(json.meta.total, 2); + + // Attempting to add a category by display text; must use slug + Category::update_crate( + tx(&req), &krate, &["Category 2".to_string()] + ).unwrap(); + assert_eq!(cnt(&mut req, "cat1"), 0); + assert_eq!(cnt(&mut req, "category-2"), 0); + } From d20cfc3ecb7169e569e853af348a3aaeed2dace0 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 21 Nov 2016 15:36:44 -0500 Subject: [PATCH 15/34] Change the header text on a category page Looking at some of the categories I have locally, I think, for example, "Command-line argument parsing Crates" is clearer than "All Crates for category 'Command-line argument parsing'" --- app/templates/category/index.hbs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/templates/category/index.hbs b/app/templates/category/index.hbs index b7d88bde3e4..26539a5013f 100644 --- a/app/templates/category/index.hbs +++ b/app/templates/category/index.hbs @@ -2,8 +2,7 @@
      -

      All Crates

      -

      for category '{{ category.category }}'

      +

      {{ category.category }} Crates

      From 9c36d770db72e4d557feefd13cf02ca6b7513eda Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 21 Nov 2016 16:30:40 -0500 Subject: [PATCH 16/34] Return warnings about invalid categories --- src/category.rs | 15 ++++++--- src/krate.rs | 16 +++++++--- src/tests/all.rs | 7 +++++ src/tests/category.rs | 8 +++-- src/tests/http-data/krate_good_categories | 21 +++++++++++++ src/tests/http-data/krate_ignored_categories | 21 +++++++++++++ src/tests/krate.rs | 32 +++++++++++++++++++- 7 files changed, 108 insertions(+), 12 deletions(-) create mode 100644 src/tests/http-data/krate_good_categories create mode 100644 src/tests/http-data/krate_ignored_categories diff --git a/src/category.rs b/src/category.rs index d699c9f924e..3d7ada56025 100644 --- a/src/category.rs +++ b/src/category.rs @@ -63,16 +63,23 @@ impl Category { pub fn update_crate(conn: &GenericConnection, krate: &Crate, - categories: &[String]) -> CargoResult<()> { + categories: &[String]) -> CargoResult> { let old_categories = try!(krate.categories(conn)); let old_categories_ids: HashSet<_> = old_categories.iter().map(|cat| { cat.id }).collect(); // If a new category specified is not in the database, filter - // it out and don't add it. + // it out and don't add it. Return it to be able to warn about it. + let mut invalid_categories = vec![]; let new_categories: Vec = categories.iter().flat_map(|c| { - Category::find_by_slug(conn, &c).ok() + match Category::find_by_slug(conn, &c) { + Ok(cat) => Some(cat), + Err(_) => { + invalid_categories.push(c.to_string()); + None + }, + } }).collect(); let new_categories_ids: HashSet<_> = new_categories.iter().map(|cat| { @@ -106,7 +113,7 @@ impl Category { &[])); } - Ok(()) + Ok(invalid_categories) } } diff --git a/src/krate.rs b/src/krate.rs index 5a67d5c22b6..f6be3c67fe4 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -739,8 +739,16 @@ pub fn new(req: &mut Request) -> CargoResult { // Update all keywords for this crate try!(Keyword::update_crate(try!(req.tx()), &krate, &keywords)); - // Update all categories for this crate - try!(Category::update_crate(try!(req.tx()), &krate, &categories)); + // Update all categories for this crate, collecting any invalid categories + // in order to be able to warn about them + let ignored_invalid_categories = try!( + Category::update_crate(try!(req.tx()), &krate, &categories) + ); + let warnings: Vec = + ignored_invalid_categories.iter().map(|category| { + format!("'{}' is not a recognized category name \ + and has been ignored.", category) + }).collect(); // Upload the crate to S3 let mut handle = req.app().handle(); @@ -799,8 +807,8 @@ pub fn new(req: &mut Request) -> CargoResult { bomb.path = None; #[derive(RustcEncodable)] - struct R { krate: EncodableCrate } - Ok(req.json(&R { krate: krate.minimal_encodable() })) + struct R { krate: EncodableCrate, warnings: Vec } + Ok(req.json(&R { krate: krate.minimal_encodable(), warnings: warnings })) } fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> { diff --git a/src/tests/all.rs b/src/tests/all.rs index b175b891d27..a97503cfa91 100755 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -283,6 +283,13 @@ fn new_req_with_keywords(app: Arc, krate: Crate, version: &str, return req; } +fn new_req_with_categories(app: Arc, krate: Crate, version: &str, + cats: Vec) -> MockRequest { + let mut req = ::req(app, Method::Put, "/api/v1/crates/new"); + req.with_body(&new_req_body(krate, version, Vec::new(), Vec::new(), cats)); + return req; +} + fn new_req_body_foo_version_2() -> Vec { new_req_body(::krate("foo"), "2.0.0", Vec::new(), Vec::new(), Vec::new()) } diff --git a/src/tests/category.rs b/src/tests/category.rs index a9f088ba03e..fe7fe932427 100644 --- a/src/tests/category.rs +++ b/src/tests/category.rs @@ -94,9 +94,11 @@ fn update_crate() { assert_eq!(cnt(&mut req, "category-2"), 0); // Attempting to add one valid category and one invalid category - Category::update_crate(tx(&req), &krate, &["cat1".to_string(), - "catnope".to_string()]).unwrap(); - + let invalid_categories = Category::update_crate( + tx(&req), &krate, &["cat1".to_string(), + "catnope".to_string()] + ).unwrap(); + assert_eq!(invalid_categories, vec!["catnope".to_string()]); assert_eq!(cnt(&mut req, "cat1"), 1); assert_eq!(cnt(&mut req, "category-2"), 0); diff --git a/src/tests/http-data/krate_good_categories b/src/tests/http-data/krate_good_categories new file mode 100644 index 00000000000..97520e5859d --- /dev/null +++ b/src/tests/http-data/krate_good_categories @@ -0,0 +1,21 @@ +===REQUEST 331 +PUT http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0.crate HTTP/1.1 +Accept: */* +Proxy-Connection: Keep-Alive +Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk= +Content-Length: 0 +Host: alexcrichton-test.s3.amazonaws.com +Content-Type: application/x-tar +Date: Sun, 28 Jun 2015 14:07:17 -0700 + + +===RESPONSE 258 +HTTP/1.1 200 +x-amz-request-id: CB0E925D8E3AB3E8 +x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g= +date: Sun, 28 Jun 2015 21:07:51 GMT +etag: "d41d8cd98f00b204e9800998ecf8427e" +content-length: 0 +server: AmazonS3 + + diff --git a/src/tests/http-data/krate_ignored_categories b/src/tests/http-data/krate_ignored_categories new file mode 100644 index 00000000000..97520e5859d --- /dev/null +++ b/src/tests/http-data/krate_ignored_categories @@ -0,0 +1,21 @@ +===REQUEST 331 +PUT http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0.crate HTTP/1.1 +Accept: */* +Proxy-Connection: Keep-Alive +Authorization: AWS AKIAJF3GEK7N44BACDZA:GDxGb6r3SIqo9wXuzHrgMNWekwk= +Content-Length: 0 +Host: alexcrichton-test.s3.amazonaws.com +Content-Type: application/x-tar +Date: Sun, 28 Jun 2015 14:07:17 -0700 + + +===RESPONSE 258 +HTTP/1.1 200 +x-amz-request-id: CB0E925D8E3AB3E8 +x-amz-id-2: SiaMwszM1p2TzXlLauvZ6kRKcUCg7HoyBW29vts42w9ArrLwkJWl8vuvPuGFkpM6XGH+YXN852g= +date: Sun, 28 Jun 2015 21:07:51 GMT +etag: "d41d8cd98f00b204e9800998ecf8427e" +content-length: 0 +server: AmazonS3 + + diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 68dca89c738..7b05ff4d922 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -26,7 +26,7 @@ struct CrateMeta { total: i32 } #[derive(RustcDecodable)] struct GitCrate { name: String, vers: String, deps: Vec, cksum: String } #[derive(RustcDecodable)] -struct GoodCrate { krate: EncodableCrate } +struct GoodCrate { krate: EncodableCrate, warnings: Vec } #[derive(RustcDecodable)] struct CrateResponse { krate: EncodableCrate, versions: Vec, keywords: Vec } #[derive(RustcDecodable)] @@ -884,6 +884,36 @@ fn bad_keywords() { } } +#[test] +fn good_categories() { + let (_b, app, middle) = ::app(); + let krate = ::krate("foo"); + let cats = vec!["cat1".into()]; + let mut req = ::new_req_with_categories(app, krate, "1.0.0", cats); + ::mock_category(&mut req, "cat1", "cat1"); + ::mock_user(&mut req, ::user("foo")); + let mut response = ok_resp!(middle.call(&mut req)); + let json: GoodCrate = ::json(&mut response); + assert_eq!(json.krate.name, "foo"); + assert_eq!(json.krate.max_version, "1.0.0"); + assert_eq!(json.warnings.len(), 0); +} + +#[test] +fn ignored_categories() { + let (_b, app, middle) = ::app(); + let krate = ::krate("foo"); + let cats = vec!["bar".into()]; + let mut req = ::new_req_with_categories(app, krate, "1.0.0", cats); + ::mock_user(&mut req, ::user("foo")); + let mut response = ok_resp!(middle.call(&mut req)); + let json: GoodCrate = ::json(&mut response); + assert_eq!(json.krate.name, "foo"); + assert_eq!(json.krate.max_version, "1.0.0"); + assert_eq!(json.warnings, vec!["\'bar\' is not a recognized category name \ + and has been ignored.".to_string()]); +} + #[test] fn reverse_dependencies() { let (_b, app, middle) = ::app(); From 27b8f95d746a5f5e89b90d013f95598d496e5e87 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Fri, 18 Nov 2016 17:44:46 -0500 Subject: [PATCH 17/34] Add some categories with subcategories --- src/categories.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/categories.txt b/src/categories.txt index 1133607f2db..c54b4e8a577 100644 --- a/src/categories.txt +++ b/src/categories.txt @@ -1,3 +1,5 @@ -test Test -attempt Attempt -node.js-ruby-ffi Node.js/Ruby FFI +development-tools Development Tools +development-tools::testing Development Tools::Testing +libraries Libraries +libraries::async Libraries::Async +libraries::date-and-time Libraries::Date and Time From d421dbdca1e37667210613a5b15c5359e97490ac Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Sat, 19 Nov 2016 13:33:57 -0500 Subject: [PATCH 18/34] Only show top level categories on the categories index page --- src/category.rs | 25 ++++++++++++++++++++----- src/tests/category.rs | 7 +++++++ 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/src/category.rs b/src/category.rs index 3d7ada56025..e6bce2c45ac 100644 --- a/src/category.rs +++ b/src/category.rs @@ -115,6 +115,18 @@ impl Category { Ok(invalid_categories) } + + pub fn count_toplevel(conn: &GenericConnection) -> CargoResult { + let sql = format!("\ + SELECT COUNT(*) \ + FROM {} \ + WHERE category NOT LIKE '%::%'", + Model::table_name(None:: + )); + let stmt = try!(conn.prepare(&sql)); + let rows = try!(stmt.query(&[])); + Ok(rows.iter().next().unwrap().get("count")) + } } impl Model for Category { @@ -141,10 +153,13 @@ pub fn index(req: &mut Request) -> CargoResult { _ => "ORDER BY category ASC", }; - // Collect all the categories - let stmt = try!(conn.prepare(&format!("SELECT * FROM categories {} \ - LIMIT $1 OFFSET $2", - sort_sql))); + // Collect all the top-level categories + let stmt = try!(conn.prepare(&format!( + "SELECT * FROM categories \ + WHERE category NOT LIKE '%::%' {} \ + LIMIT $1 OFFSET $2", + sort_sql + ))); let categories: Vec<_> = try!(stmt.query(&[&limit, &offset])) .iter() @@ -155,7 +170,7 @@ pub fn index(req: &mut Request) -> CargoResult { .collect(); // Query for the total count of categories - let total = try!(Category::count(conn)); + let total = try!(Category::count_toplevel(conn)); #[derive(RustcEncodable)] struct R { categories: Vec, meta: Meta } diff --git a/src/tests/category.rs b/src/tests/category.rs index fe7fe932427..42650937e7c 100644 --- a/src/tests/category.rs +++ b/src/tests/category.rs @@ -16,14 +16,21 @@ struct GoodCategory { category: EncodableCategory } fn index() { let (_b, app, middle) = ::app(); let mut req = ::req(app, Method::Get, "/api/v1/categories"); + + // List 0 categories if none exist let mut response = ok_resp!(middle.call(&mut req)); let json: CategoryList = ::json(&mut response); assert_eq!(json.categories.len(), 0); assert_eq!(json.meta.total, 0); + // Create a category and a subcategory ::mock_category(&mut req, "foo", "foo"); + ::mock_category(&mut req, "foo::bar", "foo::bar"); + let mut response = ok_resp!(middle.call(&mut req)); let json: CategoryList = ::json(&mut response); + + // Only the top-level categories should be on the page assert_eq!(json.categories.len(), 1); assert_eq!(json.meta.total, 1); assert_eq!(json.categories[0].category, "foo"); From 82f810c5b1de21428f44fb07acc0cfb62d97ecd7 Mon Sep 17 00:00:00 2001 From: "Carol (Nichols || Goulding)" Date: Mon, 21 Nov 2016 12:17:13 -0500 Subject: [PATCH 19/34] Show subcategories on a category's page --- app/models/category.js | 2 ++ app/templates/category/index.hbs | 19 ++++++++++++ src/category.rs | 50 ++++++++++++++++++++++++++++---- src/tests/category.rs | 16 ++++++++-- 4 files changed, 80 insertions(+), 7 deletions(-) diff --git a/app/models/category.js b/app/models/category.js index 24efc91cd18..53ea93c0653 100644 --- a/app/models/category.js +++ b/app/models/category.js @@ -6,5 +6,7 @@ export default DS.Model.extend({ created_at: DS.attr('date'), crates_cnt: DS.attr('number'), + subcategories: DS.attr(), + crates: DS.hasMany('crate', { async: true }) }); diff --git a/app/templates/category/index.hbs b/app/templates/category/index.hbs index 26539a5013f..0230dc188b6 100644 --- a/app/templates/category/index.hbs +++ b/app/templates/category/index.hbs @@ -5,6 +5,25 @@

      {{ category.category }} Crates

      +{{#if category.subcategories }} +
      +

      Subcategories

      +
      + {{#each category.subcategories as |subcategory| }} +
      +
      + {{link-to subcategory.category "category" subcategory.slug}} + + {{ format-num subcategory.crates_cnt }} + crates + +
      +
      + {{/each}} +
      +
      +{{/if}} +