Skip to content

Commit 1ed2d25

Browse files
Merge #1255
1255: More MVC Refactoring r=carols10cents This PR splits several existing source files into the MVC submodules. Here is a summary of the changes per commit: * Split `krate.rs` * Moved `WithCount` under `models::helpers`. * Moved `follow` into its own model. * Added an `ok_true()` function under `controllers::helpers` that returns a json result of `{ok: true}`. There are several additional places under `user` and `version` where this can be used as they are moved over. * Split `badge.rs` - straightforward * Extract `rights.rs` into a model used by `User` * Moved the `Rights` enum to its own file under models. * Moved `rights` from a free-standing function to a method on `User`. * Split `owner.rs` * This splits the functionality into two models: `Owner` and `Team`. * `Team::github_url()` was moved to `github::team_url` to be alongside other Github related code. * Split `crate_owner_invitations` - straightforward, and removes some duplicate `InvitationResponse` structs from the tests.
2 parents 4eaafba + f69ccb8 commit 1ed2d25

30 files changed

+483
-493
lines changed
Lines changed: 5 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -1,70 +1,10 @@
1-
use chrono::NaiveDateTime;
2-
use conduit::{Request, Response};
3-
use diesel::prelude::*;
4-
use serde_json;
5-
6-
use db::RequestTransaction;
7-
use user::RequestUser;
8-
use util::errors::{human, CargoResult};
9-
use util::RequestUtils;
10-
11-
use models::{CrateOwner, OwnerKind};
12-
use schema::{crate_owner_invitations, crate_owners, crates, users};
13-
14-
/// The model representing a row in the `crate_owner_invitations` database table.
15-
#[derive(Clone, Copy, Debug, PartialEq, Eq, Identifiable, Queryable)]
16-
#[primary_key(invited_user_id, crate_id)]
17-
pub struct CrateOwnerInvitation {
18-
pub invited_user_id: i32,
19-
pub invited_by_user_id: i32,
20-
pub crate_id: i32,
21-
pub created_at: NaiveDateTime,
22-
}
23-
24-
#[derive(Insertable, Clone, Copy, Debug)]
25-
#[table_name = "crate_owner_invitations"]
26-
pub struct NewCrateOwnerInvitation {
27-
pub invited_user_id: i32,
28-
pub invited_by_user_id: i32,
29-
pub crate_id: i32,
30-
}
1+
use super::prelude::*;
312

32-
impl CrateOwnerInvitation {
33-
pub fn invited_by_username(&self, conn: &PgConnection) -> String {
34-
users::table
35-
.find(self.invited_by_user_id)
36-
.select(users::gh_login)
37-
.first(&*conn)
38-
.unwrap_or_else(|_| String::from("(unknown username)"))
39-
}
40-
41-
pub fn crate_name(&self, conn: &PgConnection) -> String {
42-
crates::table
43-
.find(self.crate_id)
44-
.select(crates::name)
45-
.first(&*conn)
46-
.unwrap_or_else(|_| String::from("(unknown crate name)"))
47-
}
48-
49-
pub fn encodable(self, conn: &PgConnection) -> EncodableCrateOwnerInvitation {
50-
EncodableCrateOwnerInvitation {
51-
invited_by_username: self.invited_by_username(conn),
52-
crate_name: self.crate_name(conn),
53-
crate_id: self.crate_id,
54-
created_at: self.created_at,
55-
}
56-
}
57-
}
3+
use serde_json;
584

59-
/// The serialization format for the `CrateOwnerInvitation` model.
60-
#[derive(Deserialize, Serialize, Debug)]
61-
pub struct EncodableCrateOwnerInvitation {
62-
pub invited_by_username: String,
63-
pub crate_name: String,
64-
pub crate_id: i32,
65-
#[serde(with = "::util::rfc3339")]
66-
pub created_at: NaiveDateTime,
67-
}
5+
use models::{CrateOwner, CrateOwnerInvitation, OwnerKind};
6+
use schema::{crate_owner_invitations, crate_owners};
7+
use views::{EncodableCrateOwnerInvitation, InvitationResponse};
688

699
/// Handles the `GET /me/crate_owner_invitations` route.
7010
pub fn list(req: &mut Request) -> CargoResult<Response> {
@@ -92,12 +32,6 @@ struct OwnerInvitation {
9232
crate_owner_invite: InvitationResponse,
9333
}
9434

95-
#[derive(Deserialize, Serialize, Debug, Copy, Clone)]
96-
pub struct InvitationResponse {
97-
pub crate_id: i32,
98-
pub accepted: bool,
99-
}
100-
10135
/// Handles the `PUT /me/crate_owner_invitations/:crate_id` route.
10236
pub fn handle_invite(req: &mut Request) -> CargoResult<Response> {
10337
let conn = &*req.db_conn()?;
@@ -175,27 +109,3 @@ fn decline_invite(
175109
crate_owner_invitation: crate_invite,
176110
}))
177111
}
178-
179-
#[cfg(test)]
180-
mod tests {
181-
use super::*;
182-
use chrono::NaiveDate;
183-
use serde_json;
184-
185-
#[test]
186-
fn crate_owner_invitation_serializes_to_rfc3339() {
187-
let inv = EncodableCrateOwnerInvitation {
188-
invited_by_username: "".to_string(),
189-
crate_name: "".to_string(),
190-
crate_id: 123,
191-
created_at: NaiveDate::from_ymd(2017, 1, 6).and_hms(14, 23, 11),
192-
};
193-
let json = serde_json::to_string(&inv).unwrap();
194-
assert!(
195-
json.as_str()
196-
.find(r#""created_at":"2017-01-06T14:23:11+00:00""#)
197-
.is_some()
198-
);
199-
}
200-
201-
}

src/controllers/helpers/mod.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,15 @@
1+
use conduit::Response;
2+
use util::{json_response, CargoResult};
3+
14
pub mod pagination;
25

36
pub use self::pagination::Paginate;
7+
8+
pub fn ok_true() -> CargoResult<Response> {
9+
#[derive(Serialize)]
10+
struct R {
11+
ok: bool,
12+
}
13+
14+
Ok(json_response(&R { ok: true }))
15+
}

src/krate/downloads.rs renamed to src/controllers/krate/downloads.rs

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,13 @@
44
//! download counts are located in `krate::downloads`.
55
66
use std::cmp;
7-
8-
use conduit::{Request, Response};
9-
use conduit_router::RequestParams;
10-
use diesel::prelude::*;
11-
12-
use db::RequestTransaction;
13-
use util::{CargoResult, RequestUtils};
7+
use controllers::prelude::*;
148

159
use views::EncodableVersionDownload;
1610
use models::{Crate, Version, VersionDownload};
17-
use schema::*;
11+
use schema::version_downloads;
1812

19-
use super::to_char;
13+
use models::krate::to_char;
2014

2115
/// Handles the `GET /crates/:crate_id/downloads` route.
2216
pub fn downloads(req: &mut Request) -> CargoResult<Response> {

src/krate/follow.rs renamed to src/controllers/krate/follow.rs

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
11
//! Endpoints for managing a per user list of followed crates
22
3-
use conduit::{Request, Response};
4-
use conduit_router::RequestParams;
53
use diesel::associations::Identifiable;
6-
use diesel::prelude::*;
74
use diesel;
85

9-
use db::RequestTransaction;
10-
use user::RequestUser;
11-
use util::{CargoResult, RequestUtils};
12-
6+
use controllers::prelude::*;
137
use models::{Crate, Follow};
148
use schema::*;
159

@@ -32,23 +26,17 @@ pub fn follow(req: &mut Request) -> CargoResult<Response> {
3226
.values(&follow)
3327
.on_conflict_do_nothing()
3428
.execute(&*conn)?;
35-
#[derive(Serialize)]
36-
struct R {
37-
ok: bool,
38-
}
39-
Ok(req.json(&R { ok: true }))
29+
30+
ok_true()
4031
}
4132

4233
/// Handles the `DELETE /crates/:crate_id/follow` route.
4334
pub fn unfollow(req: &mut Request) -> CargoResult<Response> {
4435
let follow = follow_target(req)?;
4536
let conn = req.db_conn()?;
4637
diesel::delete(&follow).execute(&*conn)?;
47-
#[derive(Serialize)]
48-
struct R {
49-
ok: bool,
50-
}
51-
Ok(req.json(&R { ok: true }))
38+
39+
ok_true()
5240
}
5341

5442
/// Handles the `GET /crates/:crate_id/following` route.
@@ -58,6 +46,7 @@ pub fn following(req: &mut Request) -> CargoResult<Response> {
5846
let follow = follow_target(req)?;
5947
let conn = req.db_conn()?;
6048
let following = diesel::select(exists(follows::table.find(follow.id()))).get_result(&*conn)?;
49+
6150
#[derive(Serialize)]
6251
struct R {
6352
following: bool,

src/krate/metadata.rs renamed to src/controllers/krate/metadata.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,15 @@
44
//! index or cached metadata which was extracted (client side) from the
55
//! `Cargo.toml` file.
66
7-
use conduit::{Request, Response};
8-
use conduit_router::RequestParams;
9-
use diesel::prelude::*;
10-
117
use app::RequestApp;
12-
use db::RequestTransaction;
13-
use util::{human, CargoResult, RequestUtils};
148

9+
use controllers::prelude::*;
1510
use views::{EncodableCategory, EncodableCrate, EncodableDependency, EncodableKeyword,
1611
EncodableVersion};
1712
use models::{Category, Crate, CrateCategory, CrateDownload, CrateKeyword, Keyword, Version};
1813
use schema::*;
1914

20-
use super::ALL_COLUMNS;
15+
use models::krate::ALL_COLUMNS;
2116

2217
/// Handles the `GET /summary` route.
2318
pub fn summary(req: &mut Request) -> CargoResult<Response> {

src/controllers/krate/mod.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pub mod search;
2+
pub mod publish;
3+
pub mod owners;
4+
pub mod follow;
5+
pub mod downloads;
6+
pub mod metadata;

src/krate/owners.rs renamed to src/controllers/krate/owners.rs

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,8 @@
11
//! All routes related to managing owners of a crate
22
3-
use conduit::{Request, Response};
4-
use conduit_router::RequestParams;
5-
use diesel::prelude::*;
63
use serde_json;
74

8-
use app::RequestApp;
9-
use db::RequestTransaction;
10-
use owner::rights;
11-
use user::RequestUser;
12-
use util::{human, CargoResult, RequestUtils};
13-
5+
use controllers::prelude::*;
146
use views::EncodableOwner;
157
use models::{Crate, Owner, Rights, Team, User};
168

@@ -85,7 +77,7 @@ fn modify_owners(req: &mut Request, add: bool) -> CargoResult<Response> {
8577
let krate = Crate::by_name(&req.params()["crate_id"]).first::<Crate>(&*conn)?;
8678
let owners = krate.owners(&conn)?;
8779

88-
match rights(req.app(), &owners, user)? {
80+
match user.rights(req.app(), &owners)? {
8981
Rights::Full => {}
9082
// Yes!
9183
Rights::Publish => {

src/krate/publish.rs renamed to src/controllers/krate/publish.rs

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,23 +4,17 @@ use std::cmp;
44
use std::collections::HashMap;
55
use std::sync::Arc;
66

7-
use conduit::{Request, Response};
8-
use diesel::prelude::*;
97
use hex::ToHex;
108
use serde_json;
119

12-
use app::RequestApp;
13-
use db::RequestTransaction;
1410
use dependency;
1511
use git;
16-
use owner::rights;
1712
use render;
18-
use user::RequestUser;
1913
use util::{read_fill, read_le_u32};
20-
use util::{human, internal, CargoResult, ChainError, RequestUtils};
14+
use util::{internal, ChainError};
2115

22-
use views::EncodableCrate;
23-
use views::EncodableCrateUpload;
16+
use controllers::prelude::*;
17+
use views::{EncodableCrate, EncodableCrateUpload};
2418
use models::{Badge, Category, Keyword, NewCrate, NewVersion, Rights, User};
2519

2620
/// Handles the `PUT /crates/new` route.
@@ -77,7 +71,7 @@ pub fn publish(req: &mut Request) -> CargoResult<Response> {
7771
let krate = persist.create_or_update(&conn, license_file, user.id)?;
7872

7973
let owners = krate.owners(&conn)?;
80-
if rights(req.app(), &owners, &user)? < Rights::Publish {
74+
if user.rights(req.app(), &owners)? < Rights::Publish {
8175
return Err(human(
8276
"this crate exists but you don't seem to be an owner. \
8377
If you believe this is a mistake, perhaps you need \

src/krate/search.rs renamed to src/controllers/krate/search.rs

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,14 @@
11
//! Endpoint for searching and discovery functionality
22
3-
use conduit::{Request, Response};
4-
use diesel::prelude::*;
53
use diesel_full_text_search::*;
64

7-
use db::RequestTransaction;
85
use controllers::helpers::Paginate;
9-
use user::RequestUser;
10-
use util::{CargoResult, RequestUtils};
11-
6+
use controllers::prelude::*;
127
use views::EncodableCrate;
138
use models::{Badge, Crate, OwnerKind, Version};
149
use schema::*;
1510

16-
use super::{canon_crate_name, ALL_COLUMNS};
11+
use models::krate::{canon_crate_name, ALL_COLUMNS};
1712

1813
/// Handles the `GET /crates` route.
1914
/// Returns a list of crates. Called in a variety of scenarios in the

src/controllers/mod.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,19 @@
22

33
mod prelude {
44
pub use diesel::prelude::*;
5+
pub use super::helpers::ok_true;
56

7+
pub use app::RequestApp;
68
pub use conduit::{Request, Response};
79
pub use conduit_router::RequestParams;
810
pub use db::RequestTransaction;
9-
pub use util::{CargoResult, RequestUtils};
11+
pub use user::RequestUser;
12+
pub use util::{human, CargoResult, RequestUtils};
1013
}
1114

1215
pub mod helpers;
1316

1417
pub mod category;
18+
pub mod crate_owner_invitation;
1519
pub mod keyword;
20+
pub mod krate;

src/github.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,12 @@ pub fn token(token: String) -> Token {
9393
token_type: String::new(),
9494
}
9595
}
96+
97+
pub fn team_url(login: &str) -> String {
98+
let mut login_pieces = login.split(':');
99+
login_pieces.next();
100+
format!(
101+
"https://github.com/{}",
102+
login_pieces.next().expect("org failed"),
103+
)
104+
}

0 commit comments

Comments
 (0)