Drop Email column (merge and deploy the fix for #1888 first!)

migrations/20140924113530_dumped_migration_1/up.sql

@@ -3,4 +3,5 @@ CREATE TABLE users (
             email           VARCHAR NOT NULL UNIQUE,
             gh_access_token VARCHAR NOT NULL,
             api_token       VARCHAR NOT NULL
-        );
+        );
+

migrations/20161115111957_dumped_migration_119/up.sql

@@ -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();
+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

@@ -0,0 +1 @@
+-- Not reversible

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

@@ -0,0 +1 @@
+ALTER TABLE users DROP COLUMN email CASCADE;

src/bin/transfer-crates.rs

@@ -7,7 +7,7 @@
 
 use cargo_registry::{
     db,
-    models::{user, Crate, OwnerKind, User},
+    models::{Crate, OwnerKind, User},
     schema::{crate_owners, crates, users},
 };
 use std::{
@@ -16,7 +16,6 @@ use std::{
     process::exit,
 };
 
-use cargo_registry::models::user::UserNoEmailType;
 use diesel::prelude::*;
 
 fn main() {
@@ -45,16 +44,12 @@ fn transfer(conn: &PgConnection) {
     };
 
     let from = users::table
-        .select(user::ALL_COLUMNS)
         .filter(users::gh_login.eq(from))
-        .first::<UserNoEmailType>(conn)
-        .map(User::from)
+        .first::<User>(conn)
         .unwrap();
     let to = users::table
-        .select(user::ALL_COLUMNS)
         .filter(users::gh_login.eq(to))
-        .first::<UserNoEmailType>(conn)
-        .map(User::from)
+        .first::<User>(conn)
         .unwrap();
 
     if from.gh_id != to.gh_id {

src/controllers/krate/metadata.rs

@@ -5,11 +5,9 @@
 //! `Cargo.toml` file.
 
 use crate::controllers::prelude::*;
-use crate::models::user;
-use crate::models::user::UserNoEmailType;
 use crate::models::{
     Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads,
-    Version,
+    User, Version,
 };
 use crate::schema::*;
 use crate::views::{
@@ -107,10 +105,10 @@ pub fn show(req: &mut dyn Request) -> AppResult<Response> {
     let conn = req.db_conn()?;
     let krate = Crate::by_name(name).first::<Crate>(&*conn)?;
 
-    let mut versions_and_publishers: Vec<(Version, Option<UserNoEmailType>)> = krate
+    let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
         .all_versions()
         .left_outer_join(users::table)
-        .select((versions::all_columns, user::ALL_COLUMNS.nullable()))
+        .select((versions::all_columns, users::all_columns.nullable()))
         .load(&*conn)?;
     versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
     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> {
         ),
         versions: versions_and_publishers
             .into_iter()
-            .map(|(v, pb)| v.encodable(&krate.name, pb.map(From::from)))
+            .map(|(v, pb)| v.encodable(&krate.name, pb))
             .collect(),
         keywords: kws.into_iter().map(Keyword::encodable).collect(),
         categories: cats.into_iter().map(Category::encodable).collect(),
@@ -189,15 +187,15 @@ pub fn versions(req: &mut dyn Request) -> AppResult<Response> {
     let crate_name = &req.params()["crate_id"];
     let conn = req.db_conn()?;
     let krate = Crate::by_name(crate_name).first::<Crate>(&*conn)?;
-    let mut versions_and_publishers: Vec<(Version, Option<UserNoEmailType>)> = krate
+    let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
         .all_versions()
         .left_outer_join(users::table)
-        .select((versions::all_columns, user::ALL_COLUMNS.nullable()))
+        .select((versions::all_columns, users::all_columns.nullable()))
         .load(&*conn)?;
     versions_and_publishers.sort_by(|a, b| b.0.num.cmp(&a.0.num));
     let versions = versions_and_publishers
         .into_iter()
-        .map(|(v, pb)| v.encodable(crate_name, pb.map(From::from)))
+        .map(|(v, pb)| v.encodable(crate_name, pb))
         .collect();
 
     #[derive(Serialize)]
@@ -229,11 +227,11 @@ pub fn reverse_dependencies(req: &mut dyn Request) -> AppResult<Response> {
         .select((
             versions::all_columns,
             crates::name,
-            user::ALL_COLUMNS.nullable(),
+            users::all_columns.nullable(),
         ))
-        .load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?
+        .load::<(Version, String, Option<User>)>(&*conn)?
         .into_iter()
-        .map(|(version, krate_name, user)| version.encodable(&krate_name, user.map(From::from)))
+        .map(|(version, krate_name, user)| version.encodable(&krate_name, user))
         .collect();
 
     #[derive(Serialize)]

src/controllers/user/me.rs

@@ -7,7 +7,6 @@ use crate::email;
 use crate::util::bad_request;
 use crate::util::errors::AppError;
 
-use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
 use crate::models::{CrateOwner, Email, Follow, NewEmail, OwnerKind, User, Version};
 use crate::schema::{crate_owners, crates, emails, follows, users, versions};
 use crate::views::{EncodableMe, EncodableVersion, OwnedCrate};
@@ -32,12 +31,12 @@ pub fn me(req: &mut dyn Request) -> AppResult<Response> {
         .find(user_id)
         .left_join(emails::table)
         .select((
-            ALL_COLUMNS,
+            users::all_columns,
             emails::verified.nullable(),
             emails::email.nullable(),
             emails::token_generated_at.nullable().is_not_null(),
         ))
-        .first::<(UserNoEmailType, Option<bool>, Option<String>, bool)>(&*conn)?;
+        .first::<(User, Option<bool>, Option<String>, bool)>(&*conn)?;
 
     let owned_crates = crate_owners::table
         .inner_join(crates::table)
@@ -56,13 +55,8 @@ pub fn me(req: &mut dyn Request) -> AppResult<Response> {
 
     let verified = verified.unwrap_or(false);
     let verification_sent = verified || verification_sent;
-    //  PR  ::  https://github.com/rust-lang/crates.io/pull/1891
-    //          Will modify this so that we don't need this kind of conversion anymore...
-    //          In fact, the PR will use the email that we obtained from the above SQL queries
-    //          and pass it along the encodable_private function below.
-
     Ok(req.json(&EncodableMe {
-        user: User::from(user).encodable_private(email, verified, verification_sent),
+        user: user.encodable_private(email, verified, verification_sent),
         owned_crates,
     }))
 }
@@ -80,17 +74,19 @@ pub fn updates(req: &mut dyn Request) -> AppResult<Response> {
         .left_outer_join(users::table)
         .filter(crates::id.eq(any(followed_crates)))
         .order(versions::created_at.desc())
-        .select((versions::all_columns, crates::name, ALL_COLUMNS.nullable()))
+        .select((
+            versions::all_columns,
+            crates::name,
+            users::all_columns.nullable(),
+        ))
         .paginate(&req.query())?
-        .load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?;
+        .load::<(Version, String, Option<User>)>(&*conn)?;
 
     let more = data.next_page_params().is_some();
 
     let versions = data
         .into_iter()
-        .map(|(version, crate_name, published_by)| {
-            version.encodable(&crate_name, published_by.map(From::from))
-        })
+        .map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by))
         .collect();
 
     #[derive(Serialize)]
@@ -111,10 +107,10 @@ pub fn updates(req: &mut dyn Request) -> AppResult<Response> {
 /// Handles the `PUT /user/:user_id` route.
 pub fn update_user(req: &mut dyn Request) -> AppResult<Response> {
     use self::emails::user_id;
-    use self::users::dsl::{email, gh_login, users};
-    use diesel::{insert_into, update};
+    use diesel::insert_into;
 
     let mut body = String::new();
+
     req.body().read_to_string(&mut body)?;
     let user = req.user()?;
     let name = &req.params()["user_id"];
@@ -150,10 +146,6 @@ pub fn update_user(req: &mut dyn Request) -> AppResult<Response> {
     }
 
     conn.transaction::<_, Box<dyn AppError>, _>(|| {
-        update(users.filter(gh_login.eq(&user.gh_login)))
-            .set(email.eq(user_email))
-            .execute(&*conn)?;
-
         let new_email = NewEmail {
             user_id: user.id,
             email: user_email,

src/controllers/user/other.rs

@@ -1,7 +1,5 @@
 use crate::controllers::prelude::*;
 
-use crate::models::user;
-use crate::models::user::UserNoEmailType;
 use crate::models::{OwnerKind, User};
 use crate::schema::{crate_owners, crates, users};
 use crate::views::EncodablePublicUser;
@@ -13,17 +11,16 @@ pub fn show(req: &mut dyn Request) -> AppResult<Response> {
     let name = &req.params()["user_id"].to_lowercase();
     let conn = req.db_conn()?;
     let user = users
-        .select(user::ALL_COLUMNS)
         .filter(crate::lower(gh_login).eq(name))
         .order(id.desc())
-        .first::<UserNoEmailType>(&*conn)?;
+        .first::<User>(&*conn)?;
 
     #[derive(Serialize)]
     struct R {
         user: EncodablePublicUser,
     }
     Ok(req.json(&R {
-        user: User::from(user).encodable_public(),
+        user: user.encodable_public(),
     }))
 }
 

src/controllers/user/session.rs

@@ -4,8 +4,6 @@ use crate::github;
 use conduit_cookie::RequestSession;
 use oauth2::{prelude::*, AuthorizationCode, TokenResponse};
 
-use crate::models::user;
-use crate::models::user::UserNoEmailType;
 use crate::models::{NewUser, User};
 use crate::schema::users;
 use crate::util::errors::{AppError, ReadOnlyMode};
@@ -120,23 +118,21 @@ impl GithubUser {
         NewUser::new(
             self.id,
             &self.login,
-            self.email.as_ref().map(|s| &s[..]),
             self.name.as_ref().map(|s| &s[..]),
             self.avatar_url.as_ref().map(|s| &s[..]),
             access_token,
         )
-        .create_or_update(conn)
+        .create_or_update(self.email.as_ref().map(|s| &s[..]), conn)
         .map_err(Into::into)
         .or_else(|e: Box<dyn AppError>| {
             // If we're in read only mode, we can't update their details
             // just look for an existing user
             if e.is::<ReadOnlyMode>() {
                 users::table
-                    .select(user::ALL_COLUMNS)
                     .filter(users::gh_id.eq(self.id))
-                    .first::<UserNoEmailType>(conn)
-                    .map(User::from)
-                    .map_err(|_| e)
+                    .first(conn)
+                    .optional()?
+                    .ok_or(e)
             } else {
                 Err(e)
             }

src/controllers/version/deprecated.rs

@@ -7,9 +7,7 @@
 
 use crate::controllers::prelude::*;
 
-use crate::models::user;
-use crate::models::user::UserNoEmailType;
-use crate::models::{Crate, Version};
+use crate::models::{Crate, User, Version};
 use crate::schema::*;
 use crate::views::EncodableVersion;
 
@@ -30,14 +28,12 @@ pub fn index(req: &mut dyn Request) -> AppResult<Response> {
         .select((
             versions::all_columns,
             crates::name,
-            user::ALL_COLUMNS.nullable(),
+            users::all_columns.nullable(),
         ))
         .filter(versions::id.eq(any(ids)))
-        .load::<(Version, String, Option<UserNoEmailType>)>(&*conn)?
+        .load::<(Version, String, Option<User>)>(&*conn)?
         .into_iter()
-        .map(|(version, crate_name, published_by)| {
-            version.encodable(&crate_name, published_by.map(From::from))
-        })
+        .map(|(version, crate_name, published_by)| version.encodable(&crate_name, published_by))
         .collect();
 
     #[derive(Serialize)]
@@ -54,14 +50,14 @@ pub fn show_by_id(req: &mut dyn Request) -> AppResult<Response> {
     let id = &req.params()["version_id"];
     let id = id.parse().unwrap_or(0);
     let conn = req.db_conn()?;
-    let (version, krate, published_by): (Version, Crate, Option<UserNoEmailType>) = versions::table
+    let (version, krate, published_by): (Version, Crate, Option<User>) = versions::table
         .find(id)
         .inner_join(crates::table)
         .left_outer_join(users::table)
         .select((
             versions::all_columns,
             crate::models::krate::ALL_COLUMNS,
-            user::ALL_COLUMNS.nullable(),
+            users::all_columns.nullable(),
         ))
         .first(&*conn)?;
 
@@ -70,6 +66,6 @@ pub fn show_by_id(req: &mut dyn Request) -> AppResult<Response> {
         version: EncodableVersion,
     }
     Ok(req.json(&R {
-        version: version.encodable(&krate.name, published_by.map(From::from)),
+        version: version.encodable(&krate.name, published_by),
     }))
 }

src/middleware/current_user.rs

@@ -6,7 +6,6 @@ use diesel::prelude::*;
 use crate::db::RequestTransaction;
 use crate::util::errors::{std_error, AppResult, ChainError, Unauthorized};
 
-use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
 use crate::models::ApiToken;
 use crate::models::User;
 use crate::schema::users;
@@ -33,13 +32,9 @@ impl Middleware for CurrentUser {
 
         if let Some(id) = id {
             // If it did, look for a user in the database with the given `user_id`
-            let maybe_user = users::table
-                .select(ALL_COLUMNS)
-                .find(id)
-                .first::<UserNoEmailType>(&*conn);
+            let maybe_user = users::table.find(id).first::<User>(&*conn);
             drop(conn);
             if let Ok(user) = maybe_user {
-                let user = User::from(user);
                 // Attach the `User` model from the database to the request
                 req.mut_extensions().insert(user);
                 req.mut_extensions()

src/models/krate.rs

@@ -8,8 +8,6 @@ use url::Url;
 
 use crate::app::App;
 use crate::email;
-use crate::models::user;
-use crate::models::user::UserNoEmailType;
 use crate::util::{cargo_err, AppResult};
 
 use crate::models::{
@@ -402,10 +400,9 @@ impl Crate {
         let users = CrateOwner::by_owner_kind(OwnerKind::User)
             .filter(crate_owners::crate_id.eq(self.id))
             .inner_join(users::table)
-            .select(user::ALL_COLUMNS)
-            .load::<UserNoEmailType>(conn)?
+            .select(users::all_columns)
+            .load(conn)?
             .into_iter()
-            .map(User::from)
             .map(Owner::User);
         let teams = CrateOwner::by_owner_kind(OwnerKind::Team)
             .filter(crate_owners::crate_id.eq(self.id))

src/models/owner.rs

@@ -5,7 +5,6 @@ use crate::app::App;
 use crate::github;
 use crate::util::{cargo_err, AppResult};
 
-use crate::models::user::{UserNoEmailType, ALL_COLUMNS};
 use crate::models::{Crate, Team, User};
 use crate::schema::{crate_owners, users};
 use crate::views::EncodableOwner;
@@ -72,10 +71,8 @@ impl Owner {
             )?))
         } else {
             users::table
-                .select(ALL_COLUMNS)
                 .filter(users::gh_login.eq(name))
-                .first::<UserNoEmailType>(conn)
-                .map(User::from)
+                .first(conn)
                 .map(Owner::User)
                 .map_err(|_| cargo_err(&format_args!("could not find user with login `{}`", name)))
         }