Skip to content

Commit dbe39ae

Browse files
borsbors
committed
Auto merge of #1891 - hogum:drop_email_column, r=carols10cents
Drop Email column (merge and deploy the fix for #1888 first!) #### Fixes #1889 This cleans up the uses of the field `email` from `User` by: - dropping the `email` field from `User` - clearing references to the dropped email field - migrating and updating the schema changes
2 parents f3e0511 + c7ac5cb commit dbe39ae

File tree

25 files changed

+111
-189
lines changed

25 files changed

+111
-189
lines changed

‎migrations/20140924113530_dumped_migration_1/up.sql

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,4 +3,5 @@ CREATE TABLE users (
33
email VARCHAR NOT NULL UNIQUE,
44
gh_access_token VARCHAR NOT NULL,
55
api_token VARCHAR NOT NULL
6-
);
6+
);
7+

‎migrations/20161115111957_dumped_migration_119/up.sql

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
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();
1+
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();

‎migrations/2019-11-11-162609_drop_email_from_user/down.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
-- Not reversible

‎migrations/2019-11-11-162609_drop_email_from_user/up.sql

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE users DROP COLUMN email CASCADE;

‎src/bin/transfer-crates.rs

+3-8
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
use cargo_registry::{
99
db,
10-
models::{user, Crate, OwnerKind, User},
10+
models::{Crate, OwnerKind, User},
1111
schema::{crate_owners, crates, users},
1212
};
1313
use std::{
@@ -16,7 +16,6 @@ use std::{
1616
process::exit,
1717
};
1818

19-
use cargo_registry::models::user::UserNoEmailType;
2019
use diesel::prelude::*;
2120

2221
fn main() {
@@ -45,16 +44,12 @@ fn transfer(conn: &PgConnection) {
4544
};
4645

4746
let from = users::table
48-
.select(user::ALL_COLUMNS)
4947
.filter(users::gh_login.eq(from))
50-
.first::<UserNoEmailType>(conn)
51-
.map(User::from)
48+
.first::<User>(conn)
5249
.unwrap();
5350
let to = users::table
54-
.select(user::ALL_COLUMNS)
5551
.filter(users::gh_login.eq(to))
56-
.first::<UserNoEmailType>(conn)
57-
.map(User::from)
52+
.first::<User>(conn)
5853
.unwrap();
5954

6055
if from.gh_id != to.gh_id {

‎src/controllers/krate/metadata.rs

+10-12
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,9 @@
55
//! `Cargo.toml` file.
66
77
use crate::controllers::prelude::*;
8-
use crate::models::user;
9-
use crate::models::user::UserNoEmailType;
108
use crate::models::{
119
Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads,
12-
Version,
10+
User, Version,
1311
};
1412
use crate::schema::*;
1513
use crate::views::{
@@ -107,10 +105,10 @@ pub fn show(req: &mut dyn Request) -> AppResult<Response> {
107105
let conn = req.db_conn()?;
108106
let krate = Crate::by_name(name).first::<Crate>(&*conn)?;
109107

110-
let mut versions_and_publishers: Vec<(Version, Option<UserNoEmailType>)> = krate
108+
let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
111109
.all_versions()
112110
.left_outer_join(users::table)
113-
.select((versions::all_columns, user::ALL_COLUMNS.nullable()))
111+
.select((versions::all_columns, users::all_columns.nullable()))
114112
.load(&*conn)?;
115113
versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
116114
let ids = versions_and_publishers.iter().map(|v| v.0.id).collect();
@@ -153,7 +151,7 @@ pub fn show(req: &mut dyn Request) -> AppResult<Response> {
153151
),
154152
versions: versions_and_publishers
155153
.into_iter()
156-
.map(|(v, pb)| v.encodable(&krate.name, pb.map(From::from)))
154+
.map(|(v, pb)| v.encodable(&krate.name, pb))
157155
.collect(),
158156
keywords: kws.into_iter().map(Keyword::encodable).collect(),
159157
categories: cats.into_iter().map(Category::encodable).collect(),
@@ -189,15 +187,15 @@ pub fn versions(req: &mut dyn Request) -> AppResult<Response> {
189187
let crate_name = &req.params()["crate_id"];
190188
let conn = req.db_conn()?;
191189
let krate = Crate::by_name(crate_name).first::<Crate>(&*conn)?;
192-
let mut versions_and_publishers: Vec<(Version, Option<UserNoEmailType>)> = krate
190+
let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
193191
.all_versions()
194192
.left_outer_join(users::table)
195-
.select((versions::all_columns, user::ALL_COLUMNS.nullable()))
193+
.select((versions::all_columns, users::all_columns.nullable()))
196194
.load(&*conn)?;
197195
versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
198196
let versions = versions_and_publishers
199197
.into_iter()
200-
.map(|(v, pb)| v.encodable(crate_name, pb.map(From::from)))
198+
.map(|(v, pb)| v.encodable(crate_name, pb))
201199
.collect();
202200

203201
#[derive(Serialize)]
@@ -229,11 +227,11 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> AppResult<Response> {
229227
.select((
230228
versions::all_columns,
231229
crates::name,
232-
user::ALL_COLUMNS.nullable(),
230+
users::all_columns.nullable(),
233231
))
234-
.load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?
232+
.load::<(Version, String, Option<User>)>(&*conn)?
235233
.into_iter()
236-
.map(|(version, krate_name, user)| version.encodable(&krate_name, user.map(From::from)))
234+
.map(|(version, krate_name, user)| version.encodable(&krate_name, user))
237235
.collect();
238236

239237
#[derive(Serialize)]

‎src/controllers/user/me.rs

+12-20
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ use crate::email;
77
use crate::util::bad_request;
88
use crate::util::errors::AppError;
99

10-
use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
1110
use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version};
1211
use crate::schema::{crate_owners, crates, emails, follows, users, versions};
1312
use crate::views::{EncodableMe, EncodableVersion, OwnedCrate};
@@ -32,12 +31,12 @@ pub fn me(req: &mut dyn Request) -> AppResult<Response> {
3231
.find(user_id)
3332
.left_join(emails::table)
3433
.select((
35-
ALL_COLUMNS,
34+
users::all_columns,
3635
emails::verified.nullable(),
3736
emails::email.nullable(),
3837
emails::token_generated_at.nullable().is_not_null(),
3938
))
40-
.first::<(UserNoEmailType, Option<bool>, Option<String>, bool)>(&*conn)?;
39+
.first::<(User, Option<bool>, Option<String>, bool)>(&*conn)?;
4140

4241
let owned_crates = crate_owners::table
4342
.inner_join(crates::table)
@@ -56,13 +55,8 @@ pub fn me(req: &mut dyn Request) -> AppResult<Response> {
5655

5756
let verified = verified.unwrap_or(false);
5857
let verification_sent = verified || verification_sent;
59-
// PR :: https://github.com/rust-lang/crates.io/pull/1891
60-
// Will modify this so that we don't need this kind of conversion anymore...
61-
// In fact, the PR will use the email that we obtained from the above SQL queries
62-
// and pass it along the encodable_private function below.
63-
6458
Ok(req.json(&EncodableMe {
65-
user: User::from(user).encodable_private(email, verified, verification_sent),
59+
user: user.encodable_private(email, verified, verification_sent),
6660
owned_crates,
6761
}))
6862
}
@@ -80,17 +74,19 @@ pub fn updates(req: &mut dyn Request) -> AppResult<Response> {
8074
.left_outer_join(users::table)
8175
.filter(crates::id.eq(any(followed_crates)))
8276
.order(versions::created_at.desc())
83-
.select((versions::all_columns, crates::name, ALL_COLUMNS.nullable()))
77+
.select((
78+
versions::all_columns,
79+
crates::name,
80+
users::all_columns.nullable(),
81+
))
8482
.paginate(&req.query())?
85-
.load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?;
83+
.load::<(Version, String, Option<User>)>(&*conn)?;
8684

8785
let more = data.next_page_params().is_some();
8886

8987
let versions = data
9088
.into_iter()
91-
.map(|(version, crate_name, published_by)| {
92-
version.encodable(&crate_name, published_by.map(From::from))
93-
})
89+
.map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by))
9490
.collect();
9591

9692
#[derive(Serialize)]
@@ -111,10 +107,10 @@ pub fn updates(req: &mut dyn Request) -> AppResult<Response> {
111107
/// Handles the `PUT /user/:user_id` route.
112108
pub fn update_user(req: &mut dyn Request) -> AppResult<Response> {
113109
use self::emails::user_id;
114-
use self::users::dsl::{email, gh_login, users};
115-
use diesel::{insert_into, update};
110+
use diesel::insert_into;
116111

117112
let mut body = String::new();
113+
118114
req.body().read_to_string(&mut body)?;
119115
let user = req.user()?;
120116
let name = &req.params()["user_id"];
@@ -150,10 +146,6 @@ pub fn update_user(req: &mut dyn Request) -> AppResult<Response> {
150146
}
151147

152148
conn.transaction::<_, Box<dyn AppError>, _>(|| {
153-
update(users.filter(gh_login.eq(&user.gh_login)))
154-
.set(email.eq(user_email))
155-
.execute(&*conn)?;
156-
157149
let new_email = NewEmail {
158150
user_id: user.id,
159151
email: user_email,

‎src/controllers/user/other.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
use crate::controllers::prelude::*;
22

3-
use crate::models::user;
4-
use crate::models::user::UserNoEmailType;
53
use crate::models::{OwnerKind, User};
64
use crate::schema::{crate_owners, crates, users};
75
use crate::views::EncodablePublicUser;
@@ -13,17 +11,16 @@ pub fn show(req: &mut dyn Request) -> AppResult<Response> {
1311
let name = &req.params()["user_id"].to_lowercase();
1412
let conn = req.db_conn()?;
1513
let user = users
16-
.select(user::ALL_COLUMNS)
1714
.filter(crate::lower(gh_login).eq(name))
1815
.order(id.desc())
19-
.first::<UserNoEmailType>(&*conn)?;
16+
.first::<User>(&*conn)?;
2017

2118
#[derive(Serialize)]
2219
struct R {
2320
user: EncodablePublicUser,
2421
}
2522
Ok(req.json(&R {
26-
user: User::from(user).encodable_public(),
23+
user: user.encodable_public(),
2724
}))
2825
}
2926

‎src/controllers/user/session.rs

+4-8
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ use crate::github;
44
use conduit_cookie::RequestSession;
55
use oauth2::{prelude::*, AuthorizationCode, TokenResponse};
66

7-
use crate::models::user;
8-
use crate::models::user::UserNoEmailType;
97
use crate::models::{NewUser, User};
108
use crate::schema::users;
119
use crate::util::errors::{AppError, ReadOnlyMode};
@@ -120,23 +118,21 @@ impl GithubUser {
120118
NewUser::new(
121119
self.id,
122120
&self.login,
123-
self.email.as_ref().map(|s| &s[..]),
124121
self.name.as_ref().map(|s| &s[..]),
125122
self.avatar_url.as_ref().map(|s| &s[..]),
126123
access_token,
127124
)
128-
.create_or_update(conn)
125+
.create_or_update(self.email.as_ref().map(|s| &s[..]), conn)
129126
.map_err(Into::into)
130127
.or_else(|e: Box<dyn AppError>| {
131128
// If we're in read only mode, we can't update their details
132129
// just look for an existing user
133130
if e.is::<ReadOnlyMode>() {
134131
users::table
135-
.select(user::ALL_COLUMNS)
136132
.filter(users::gh_id.eq(self.id))
137-
.first::<UserNoEmailType>(conn)
138-
.map(User::from)
139-
.map_err(|_| e)
133+
.first(conn)
134+
.optional()?
135+
.ok_or(e)
140136
} else {
141137
Err(e)
142138
}

‎src/controllers/version/deprecated.rs

+7-11
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
88
use crate::controllers::prelude::*;
99

10-
use crate::models::user;
11-
use crate::models::user::UserNoEmailType;
12-
use crate::models::{Crate, Version};
10+
use crate::models::{Crate, User, Version};
1311
use crate::schema::*;
1412
use crate::views::EncodableVersion;
1513

@@ -30,14 +28,12 @@ pub fn index(req: &mut dyn Request) -> AppResult<Response> {
3028
.select((
3129
versions::all_columns,
3230
crates::name,
33-
user::ALL_COLUMNS.nullable(),
31+
users::all_columns.nullable(),
3432
))
3533
.filter(versions::id.eq(any(ids)))
36-
.load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?
34+
.load::<(Version, String, Option<User>)>(&*conn)?
3735
.into_iter()
38-
.map(|(version, crate_name, published_by)| {
39-
version.encodable(&crate_name, published_by.map(From::from))
40-
})
36+
.map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by))
4137
.collect();
4238

4339
#[derive(Serialize)]
@@ -54,14 +50,14 @@ pub fn show_by_id(req: &mut dyn Request) -> AppResult<Response> {
5450
let id = &req.params()["version_id"];
5551
let id = id.parse().unwrap_or(0);
5652
let conn = req.db_conn()?;
57-
let (version, krate, published_by): (Version, Crate, Option<UserNoEmailType>) = versions::table
53+
let (version, krate, published_by): (Version, Crate, Option<User>) = versions::table
5854
.find(id)
5955
.inner_join(crates::table)
6056
.left_outer_join(users::table)
6157
.select((
6258
versions::all_columns,
6359
crate::models::krate::ALL_COLUMNS,
64-
user::ALL_COLUMNS.nullable(),
60+
users::all_columns.nullable(),
6561
))
6662
.first(&*conn)?;
6763

@@ -70,6 +66,6 @@ pub fn show_by_id(req: &mut dyn Request) -> AppResult<Response> {
7066
version: EncodableVersion,
7167
}
7268
Ok(req.json(&R {
73-
version: version.encodable(&krate.name, published_by.map(From::from)),
69+
version: version.encodable(&krate.name, published_by),
7470
}))
7571
}

‎src/middleware/current_user.rs

+1-6
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use diesel::prelude::*;
66
use crate::db::RequestTransaction;
77
use crate::util::errors::{std_error, AppResult, ChainError, Unauthorized};
88

9-
use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
109
use crate::models::ApiToken;
1110
use crate::models::User;
1211
use crate::schema::users;
@@ -33,13 +32,9 @@ impl Middleware for CurrentUser {
3332

3433
if let Some(id) = id {
3534
// If it did, look for a user in the database with the given `user_id`
36-
let maybe_user = users::table
37-
.select(ALL_COLUMNS)
38-
.find(id)
39-
.first::<UserNoEmailType>(&*conn);
35+
let maybe_user = users::table.find(id).first::<User>(&*conn);
4036
drop(conn);
4137
if let Ok(user) = maybe_user {
42-
let user = User::from(user);
4338
// Attach the `User` model from the database to the request
4439
req.mut_extensions().insert(user);
4540
req.mut_extensions()

‎src/models/krate.rs

+2-5
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ use url::Url;
88

99
use crate::app::App;
1010
use crate::email;
11-
use crate::models::user;
12-
use crate::models::user::UserNoEmailType;
1311
use crate::util::{cargo_err, AppResult};
1412

1513
use crate::models::{
@@ -402,10 +400,9 @@ impl Crate {
402400
let users = CrateOwner::by_owner_kind(OwnerKind::User)
403401
.filter(crate_owners::crate_id.eq(self.id))
404402
.inner_join(users::table)
405-
.select(user::ALL_COLUMNS)
406-
.load::<UserNoEmailType>(conn)?
403+
.select(users::all_columns)
404+
.load(conn)?
407405
.into_iter()
408-
.map(User::from)
409406
.map(Owner::User);
410407
let teams = CrateOwner::by_owner_kind(OwnerKind::Team)
411408
.filter(crate_owners::crate_id.eq(self.id))

‎src/models/owner.rs

+1-4
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ use crate::app::App;
55
use crate::github;
66
use crate::util::{cargo_err, AppResult};
77

8-
use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
98
use crate::models::{Crate, Team, User};
109
use crate::schema::{crate_owners, users};
1110
use crate::views::EncodableOwner;
@@ -72,10 +71,8 @@ impl Owner {
7271
)?))
7372
} else {
7473
users::table
75-
.select(ALL_COLUMNS)
7674
.filter(users::gh_login.eq(name))
77-
.first::<UserNoEmailType>(conn)
78-
.map(User::from)
75+
.first(conn)
7976
.map(Owner::User)
8077
.map_err(|_| cargo_err(&format_args!("could not find user with login `{}`", name)))
8178
}

0 commit comments

Comments
 (0)