Skip to content

Commit 0d0aed7

Browse files
smarnachsmarnach
committed
Merge remote-tracking branch 'origin' into fix-owner-checks
2 parents 7976674 + b0b4276 commit 0d0aed7

File tree

12 files changed

+433
-559
lines changed

12 files changed

+433
-559
lines changed

‎app/routes/team.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ export default Route.extend({
1616
return this.store.queryRecord('team', { team_id }).then(
1717
team => {
1818
params.team_id = team.get('id');
19+
params.include_yanked = 'n';
1920
return RSVP.hash({
2021
crates: this.store.query('crate', params),
2122
team,

‎app/routes/user.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export default Route.extend({
1515
return this.store.queryRecord('user', { user_id }).then(
1616
user => {
1717
params.user_id = user.get('id');
18+
params.include_yanked = 'n';
1819
return RSVP.hash({
1920
crates: this.store.query('crate', params),
2021
user,

‎build.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
use diesel::prelude::*;
22
use diesel_migrations::run_pending_migrations;
3-
use dotenv::dotenv;
43
use std::env;
54

65
fn main() {
76
println!("cargo:rerun-if-env-changed=TEST_DATABASE_URL");
8-
println!("cargo:rerun-if-changed=build.rs");
7+
println!("cargo:rerun-if-changed=.env");
98
println!("cargo:rerun-if-changed=migrations/");
109
if env::var("PROFILE") == Ok("debug".into()) {
11-
let _ = dotenv();
12-
if let Ok(database_url) = env::var("TEST_DATABASE_URL") {
10+
if let Ok(database_url) = dotenv::var("TEST_DATABASE_URL") {
1311
let connection = PgConnection::establish(&database_url)
1412
.expect("Could not connect to TEST_DATABASE_URL");
1513
run_pending_migrations(&connection).expect("Error running migrations");

‎migrations/2019-06-14-195059_remove_unused_indexes/down.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
CREATE INDEX index_version_num ON versions (num);
2+
CREATE INDEX path_categories_idx ON categories (path);

‎migrations/2019-06-14-195059_remove_unused_indexes/up.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
DROP INDEX path_categories_idx;
2+
DROP INDEX index_version_num;

‎src/controllers/krate/search.rs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//! Endpoint for searching and discovery functionality
22
3+
use diesel::dsl::*;
34
use diesel::sql_types::{NotNull, Nullable};
45
use diesel_full_text_search::*;
56

@@ -33,7 +34,6 @@ use crate::models::krate::{canon_crate_name, ALL_COLUMNS};
3334
/// function out to cover the different use cases, and create unit tests
3435
/// for them.
3536
pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
36-
use diesel::dsl::sql;
3737
use diesel::sql_types::{Bool, Text};
3838

3939
let conn = req.db_conn()?;
@@ -44,6 +44,10 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
4444
.map(|s| &**s)
4545
.unwrap_or("recent-downloads");
4646
let mut has_filter = false;
47+
let include_yanked = params
48+
.get("include_yanked")
49+
.map(|s| s == "yes")
50+
.unwrap_or(true);
4751

4852
let selection = (
4953
ALL_COLUMNS,
@@ -154,6 +158,15 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
154158
);
155159
}
156160

161+
if !include_yanked {
162+
has_filter = true;
163+
query = query.filter(exists(
164+
versions::table
165+
.filter(versions::crate_id.eq(crates::id))
166+
.filter(versions::yanked.eq(false)),
167+
));
168+
}
169+
157170
if sort == "downloads" {
158171
query = query.then_order_by(crates::downloads.desc())
159172
} else if sort == "recent-downloads" {

‎src/tests/all.rs

Lines changed: 21 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@ extern crate serde_json;
1717

1818
use crate::util::{Bad, RequestHelper, TestApp};
1919
use cargo_registry::{
20-
middleware::current_user::AuthenticationSource,
2120
models::{Crate, CrateOwner, Dependency, NewCategory, NewTeam, NewUser, Team, User, Version},
2221
schema::crate_owners,
2322
util::CargoResult,
24-
views::{EncodableCrate, EncodableKeyword, EncodableOwner, EncodableVersion, GoodCrate},
23+
views::{
24+
EncodableCategory, EncodableCategoryWithSubcategories, EncodableCrate, EncodableKeyword,
25+
EncodableOwner, EncodableVersion, GoodCrate,
26+
},
2527
App, Config, Env, Replica, Uploader,
2628
};
2729
use std::{
@@ -32,7 +34,6 @@ use std::{
3234
},
3335
};
3436

35-
use conduit::Request;
3637
use conduit_test::MockRequest;
3738
use diesel::prelude::*;
3839
use reqwest::{Client, Proxy};
@@ -47,26 +48,6 @@ macro_rules! t {
4748
};
4849
}
4950

50-
macro_rules! ok_resp {
51-
($e:expr) => {{
52-
let resp = t!($e);
53-
if !crate::ok_resp(&resp) {
54-
panic!("bad response: {:?}", resp.status);
55-
}
56-
resp
57-
}};
58-
}
59-
60-
macro_rules! bad_resp {
61-
($e:expr) => {{
62-
let mut resp = t!($e);
63-
match crate::bad_resp(&mut resp) {
64-
None => panic!("ok response: {:?}", resp.status),
65-
Some(b) => b,
66-
}
67-
}};
68-
}
69-
7051
mod badge;
7152
mod builders;
7253
mod categories;
@@ -110,6 +91,23 @@ pub struct OwnerTeamsResponse {
11091
teams: Vec<EncodableOwner>,
11192
}
11293
#[derive(Deserialize)]
94+
pub struct OwnersResponse {
95+
users: Vec<EncodableOwner>,
96+
}
97+
#[derive(Deserialize)]
98+
pub struct CategoryResponse {
99+
category: EncodableCategoryWithSubcategories,
100+
}
101+
#[derive(Deserialize)]
102+
pub struct CategoryListResponse {
103+
categories: Vec<EncodableCategory>,
104+
meta: CategoryMeta,
105+
}
106+
#[derive(Deserialize)]
107+
pub struct CategoryMeta {
108+
total: i32,
109+
}
110+
#[derive(Deserialize)]
113111
pub struct OkBool {
114112
ok: bool,
115113
}
@@ -254,12 +252,6 @@ fn add_team_to_crate(t: &Team, krate: &Crate, u: &User, conn: &PgConnection) ->
254252
Ok(())
255253
}
256254

257-
fn sign_in_as(req: &mut dyn Request, user: &User) {
258-
req.mut_extensions().insert(user.clone());
259-
req.mut_extensions()
260-
.insert(AuthenticationSource::SessionCookie);
261-
}
262-
263255
fn new_dependency(conn: &PgConnection, version: &Version, krate: &Crate) -> Dependency {
264256
use cargo_registry::schema::dependencies::dsl::*;
265257
use diesel::insert_into;
@@ -285,10 +277,6 @@ fn new_category<'a>(category: &'a str, slug: &'a str, description: &'a str) -> N
285277
}
286278
}
287279

288-
fn logout(req: &mut dyn Request) {
289-
req.mut_extensions().pop::<User>();
290-
}
291-
292280
#[test]
293281
fn multiple_live_references_to_the_same_connection_can_be_checked_out() {
294282
use std::ptr;

‎src/tests/category.rs

Lines changed: 71 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,8 @@
1-
use crate::{app, builders::CrateBuilder, new_category, new_user, req, RequestHelper, TestApp};
2-
use cargo_registry::{
3-
models::Category,
4-
views::{EncodableCategory, EncodableCategoryWithSubcategories},
1+
use crate::{
2+
builders::CrateBuilder, new_category, util::MockAnonymousUser, RequestHelper, TestApp,
53
};
4+
use cargo_registry::{models::Category, views::EncodableCategoryWithSubcategories};
65

7-
use conduit::{Handler, Method};
8-
9-
#[derive(Deserialize)]
10-
struct CategoryList {
11-
categories: Vec<EncodableCategory>,
12-
meta: CategoryMeta,
13-
}
14-
#[derive(Deserialize)]
15-
struct CategoryMeta {
16-
total: i32,
17-
}
18-
#[derive(Deserialize)]
19-
struct GoodCategory {
20-
category: EncodableCategory,
21-
}
226
#[derive(Deserialize)]
237
struct CategoryWithSubcategories {
248
category: EncodableCategoryWithSubcategories,
@@ -27,10 +11,9 @@ struct CategoryWithSubcategories {
2711
#[test]
2812
fn index() {
2913
let (app, anon) = TestApp::init().empty();
30-
let url = "/api/v1/categories";
3114

3215
// List 0 categories if none exist
33-
let json: CategoryList = anon.get(url).good();
16+
let json = anon.show_category_list();
3417
assert_eq!(json.categories.len(), 0);
3518
assert_eq!(json.meta.total, 0);
3619

@@ -45,7 +28,7 @@ fn index() {
4528
});
4629

4730
// Only the top-level categories should be on the page
48-
let json: CategoryList = anon.get(url).good();
31+
let json = anon.show_category_list();
4932
assert_eq!(json.categories.len(), 1);
5033
assert_eq!(json.meta.total, 1);
5134
assert_eq!(json.categories[0].category, "foo");
@@ -76,94 +59,76 @@ fn show() {
7659
#[test]
7760
#[allow(clippy::cognitive_complexity)]
7861
fn update_crate() {
79-
let (app, middle) = app();
80-
let mut req = req(Method::Get, "/api/v1/categories/foo");
81-
macro_rules! cnt {
82-
($req:expr, $cat:expr) => {{
83-
$req.with_path(&format!("/api/v1/categories/{}", $cat));
84-
let mut response = ok_resp!(middle.call($req));
85-
crate::json::<GoodCategory>(&mut response)
86-
.category
87-
.crates_cnt as usize
88-
}};
62+
// Convenience function to get the number of crates in a category
63+
fn count(anon: &MockAnonymousUser, category: &str) -> usize {
64+
let json = anon.show_category(category);
65+
json.category.crates_cnt as usize
8966
}
9067

91-
let krate = {
92-
let conn = t!(app.diesel_database.get());
93-
let user = t!(new_user("foo").create_or_update(&conn));
94-
t!(new_category("cat1", "cat1", "Category 1 crates").create_or_update(&conn));
95-
t!(new_category("Category 2", "category-2", "Category 2 crates").create_or_update(&conn));
96-
CrateBuilder::new("foo_crate", user.id).expect_build(&conn)
97-
};
68+
let (app, anon, user) = TestApp::init().with_user();
69+
let user = user.as_model();
9870

99-
// Updating with no categories has no effect
100-
Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &[]).unwrap();
101-
assert_eq!(cnt!(&mut req, "cat1"), 0);
102-
assert_eq!(cnt!(&mut req, "category-2"), 0);
103-
104-
// Happy path adding one category
105-
Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &["cat1"]).unwrap();
106-
assert_eq!(cnt!(&mut req, "cat1"), 1);
107-
assert_eq!(cnt!(&mut req, "category-2"), 0);
108-
109-
// Replacing one category with another
110-
Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &["category-2"]).unwrap();
111-
assert_eq!(cnt!(&mut req, "cat1"), 0);
112-
assert_eq!(cnt!(&mut req, "category-2"), 1);
113-
114-
// Removing one category
115-
Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &[]).unwrap();
116-
assert_eq!(cnt!(&mut req, "cat1"), 0);
117-
assert_eq!(cnt!(&mut req, "category-2"), 0);
118-
119-
// Adding 2 categories
120-
Category::update_crate(
121-
&app.diesel_database.get().unwrap(),
122-
&krate,
123-
&["cat1", "category-2"],
124-
)
125-
.unwrap();
126-
assert_eq!(cnt!(&mut req, "cat1"), 1);
127-
assert_eq!(cnt!(&mut req, "category-2"), 1);
128-
129-
// Removing all categories
130-
Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &[]).unwrap();
131-
assert_eq!(cnt!(&mut req, "cat1"), 0);
132-
assert_eq!(cnt!(&mut req, "category-2"), 0);
133-
134-
// Attempting to add one valid category and one invalid category
135-
let invalid_categories = Category::update_crate(
136-
&app.diesel_database.get().unwrap(),
137-
&krate,
138-
&["cat1", "catnope"],
139-
)
140-
.unwrap();
141-
assert_eq!(invalid_categories, vec!["catnope"]);
142-
assert_eq!(cnt!(&mut req, "cat1"), 1);
143-
assert_eq!(cnt!(&mut req, "category-2"), 0);
144-
145-
// Does not add the invalid category to the category list
146-
// (unlike the behavior of keywords)
147-
req.with_path("/api/v1/categories");
148-
let mut response = ok_resp!(middle.call(&mut req));
149-
let json: CategoryList = crate::json(&mut response);
150-
assert_eq!(json.categories.len(), 2);
151-
assert_eq!(json.meta.total, 2);
152-
153-
// Attempting to add a category by display text; must use slug
154-
Category::update_crate(&app.diesel_database.get().unwrap(), &krate, &["Category 2"]).unwrap();
155-
assert_eq!(cnt!(&mut req, "cat1"), 0);
156-
assert_eq!(cnt!(&mut req, "category-2"), 0);
157-
158-
// Add a category and its subcategory
159-
{
160-
let conn = t!(app.diesel_database.get());
161-
t!(new_category("cat1::bar", "cat1::bar", "bar crates").create_or_update(&conn,));
71+
app.db(|conn| {
72+
t!(new_category("cat1", "cat1", "Category 1 crates").create_or_update(conn));
73+
t!(new_category("Category 2", "category-2", "Category 2 crates").create_or_update(conn));
74+
let krate = CrateBuilder::new("foo_crate", user.id).expect_build(&conn);
75+
76+
// Updating with no categories has no effect
77+
Category::update_crate(conn, &krate, &[]).unwrap();
78+
assert_eq!(count(&anon, "cat1"), 0);
79+
assert_eq!(count(&anon, "category-2"), 0);
80+
81+
// Happy path adding one category
82+
Category::update_crate(conn, &krate, &["cat1"]).unwrap();
83+
assert_eq!(count(&anon, "cat1"), 1);
84+
assert_eq!(count(&anon, "category-2"), 0);
85+
86+
// Replacing one category with another
87+
Category::update_crate(conn, &krate, &["category-2"]).unwrap();
88+
assert_eq!(count(&anon, "cat1"), 0);
89+
assert_eq!(count(&anon, "category-2"), 1);
90+
91+
// Removing one category
92+
Category::update_crate(conn, &krate, &[]).unwrap();
93+
assert_eq!(count(&anon, "cat1"), 0);
94+
assert_eq!(count(&anon, "category-2"), 0);
95+
96+
// Adding 2 categories
97+
Category::update_crate(conn, &krate, &["cat1", "category-2"]).unwrap();
98+
assert_eq!(count(&anon, "cat1"), 1);
99+
assert_eq!(count(&anon, "category-2"), 1);
100+
101+
// Removing all categories
102+
Category::update_crate(conn, &krate, &[]).unwrap();
103+
assert_eq!(count(&anon, "cat1"), 0);
104+
assert_eq!(count(&anon, "category-2"), 0);
105+
106+
// Attempting to add one valid category and one invalid category
107+
let invalid_categories =
108+
Category::update_crate(conn, &krate, &["cat1", "catnope"]).unwrap();
109+
assert_eq!(invalid_categories, vec!["catnope"]);
110+
assert_eq!(count(&anon, "cat1"), 1);
111+
assert_eq!(count(&anon, "category-2"), 0);
112+
113+
// Does not add the invalid category to the category list
114+
// (unlike the behavior of keywords)
115+
let json = anon.show_category_list();
116+
assert_eq!(json.categories.len(), 2);
117+
assert_eq!(json.meta.total, 2);
118+
119+
// Attempting to add a category by display text; must use slug
120+
Category::update_crate(conn, &krate, &["Category 2"]).unwrap();
121+
assert_eq!(count(&anon, "cat1"), 0);
122+
assert_eq!(count(&anon, "category-2"), 0);
123+
124+
// Add a category and its subcategory
125+
t!(new_category("cat1::bar", "cat1::bar", "bar crates").create_or_update(conn));
162126
Category::update_crate(&conn, &krate, &["cat1", "cat1::bar"]).unwrap();
163-
}
164-
assert_eq!(cnt!(&mut req, "cat1"), 1);
165-
assert_eq!(cnt!(&mut req, "cat1::bar"), 1);
166-
assert_eq!(cnt!(&mut req, "category-2"), 0);
127+
128+
assert_eq!(count(&anon, "cat1"), 1);
129+
assert_eq!(count(&anon, "cat1::bar"), 1);
130+
assert_eq!(count(&anon, "category-2"), 0);
131+
});
167132
}
168133

169134
#[test]

0 commit comments

Comments
 (0)