Merge #1090

1090: Remove the `tokens` table r=carols10cents

This table was always one-to-one with the emails table, and we should
have always been updating it when the email was updated. This moves all
of that logic into the database, and merges the two tables.

The only reason that these tables were ever separate as far as I can
tell was to represent when the confirmation email hadn't been sent.
However, we can just do that with a nullable column.

However, having the `token` column leads to some awkward `unwrap`s in
the code where we return it, since we know that updating the `email`
always generates a new token. For that reason I have made the `token`
column `NOT NULL`, but left the timestamp nullable. Rows in the `emails`
table that did not previously have an associated `token` row will have a
`NULL` value for this column.

Author: bors-voyager[bot]

migrations/2017-09-23-182408_move_tokens_to_emails_table/down.sql

@@ -0,0 +1,17 @@
+DROP TRIGGER trigger_emails_reconfirm ON emails;
+DROP TRIGGER trigger_emails_set_token_generated_at ON emails;
+DROP FUNCTION reconfirm_email_on_email_change();
+DROP FUNCTION emails_set_token_generated_at();
+
+CREATE TABLE tokens (
+  id SERIAL PRIMARY KEY,
+  email_id INTEGER NOT NULL UNIQUE REFERENCES emails (id),
+  token TEXT NOT NULL,
+  created_at TIMESTAMP NOT NULL DEFAULT NOW()
+);
+
+INSERT INTO tokens (email_id, token, created_at)
+SELECT id, token, token_generated_at FROM emails WHERE token_generated_at IS NOT NULL;
+
+ALTER TABLE emails DROP COLUMN token;
+ALTER TABLE emails DROP COLUMN token_generated_at;

migrations/2017-09-23-182408_move_tokens_to_emails_table/up.sql

@@ -0,0 +1,30 @@
+ALTER TABLE emails ADD COLUMN token TEXT NOT NULL DEFAULT random_string(26);
+ALTER TABLE emails ADD COLUMN token_generated_at TIMESTAMP;
+UPDATE emails SET token = tokens.token, token_generated_at = tokens.created_at
+  FROM tokens WHERE tokens.email_id = emails.id;
+DROP TABLE tokens;
+
+CREATE FUNCTION emails_set_token_generated_at() RETURNS trigger AS $$
+  BEGIN
+    NEW.token_generated_at := CURRENT_TIMESTAMP;
+    RETURN NEW;
+  END
+$$ LANGUAGE plpgsql;
+
+CREATE FUNCTION reconfirm_email_on_email_change() RETURNS trigger AS $$
+  BEGIN
+    IF NEW.email IS DISTINCT FROM OLD.email THEN
+      NEW.token := random_string(26);
+      NEW.verified := false;
+    END IF;
+    RETURN NEW;
+  END
+$$ LANGUAGE plpgsql;
+
+CREATE TRIGGER trigger_emails_set_token_generated_at BEFORE
+INSERT OR UPDATE OF token ON emails
+FOR EACH ROW EXECUTE PROCEDURE emails_set_token_generated_at();
+
+CREATE TRIGGER trigger_emails_reconfirm BEFORE UPDATE
+ON emails
+FOR EACH ROW EXECUTE PROCEDURE reconfirm_email_on_email_change();

migrations/2017-09-23-191545_add_users_to_emails_foreign_key/down.sql

@@ -0,0 +1 @@
+ALTER TABLE emails DROP CONSTRAINT fk_emails_user_id;

migrations/2017-09-23-191545_add_users_to_emails_foreign_key/up.sql

@@ -0,0 +1 @@
+ALTER TABLE emails ADD CONSTRAINT fk_emails_user_id FOREIGN KEY (user_id) REFERENCES users (id);

src/schema.rs

@@ -437,6 +437,18 @@ table! {
         ///
         /// (Automatically generated by Diesel.)
         verified -> Bool,
+        /// The `token` column of the `emails` table.
+        ///
+        /// Its SQL type is `Text`.
+        ///
+        /// (Automatically generated by Diesel.)
+        token -> Text,
+        /// The `token_generated_at` column of the `emails` table.
+        ///
+        /// Its SQL type is `Nullable<Timestamp>`.
+        ///
+        /// (Automatically generated by Diesel.)
+        token_generated_at -> Nullable<Timestamp>,
     }
 }
 
@@ -558,38 +570,6 @@ table! {
     }
 }
 
-table! {
-    /// Representation of the `tokens` table.
-    ///
-    /// (Automatically generated by Diesel.)
-    tokens (id) {
-        /// The `id` column of the `tokens` table.
-        ///
-        /// Its SQL type is `Int4`.
-        ///
-        /// (Automatically generated by Diesel.)
-        id -> Int4,
-        /// The `email_id` column of the `tokens` table.
-        ///
-        /// Its SQL type is `Int4`.
-        ///
-        /// (Automatically generated by Diesel.)
-        email_id -> Int4,
-        /// The `token` column of the `tokens` table.
-        ///
-        /// Its SQL type is `Varchar`.
-        ///
-        /// (Automatically generated by Diesel.)
-        token -> Varchar,
-        /// The `created_at` column of the `tokens` table.
-        ///
-        /// Its SQL type is `Timestamp`.
-        ///
-        /// (Automatically generated by Diesel.)
-        created_at -> Timestamp,
-    }
-}
-
 table! {
     /// Representation of the `users` table.
     ///
@@ -784,22 +764,22 @@ table! {
     }
 }
 
-joinable!(follows -> users (user_id));
-joinable!(version_authors -> users (user_id));
-joinable!(crates_keywords -> keywords (keyword_id));
-joinable!(crates_categories -> categories (category_id));
 joinable!(api_tokens -> users (user_id));
 joinable!(crate_downloads -> crates (crate_id));
+joinable!(crate_owner_invitations -> crates (crate_id));
 joinable!(crate_owners -> crates (crate_id));
+joinable!(crate_owners -> teams (owner_id));
+joinable!(crate_owners -> users (owner_id));
+joinable!(crates_categories -> categories (category_id));
 joinable!(crates_categories -> crates (crate_id));
 joinable!(crates_keywords -> crates (crate_id));
+joinable!(crates_keywords -> keywords (keyword_id));
 joinable!(dependencies -> crates (crate_id));
-joinable!(follows -> crates (crate_id));
-joinable!(versions -> crates (crate_id));
 joinable!(dependencies -> versions (version_id));
+joinable!(emails -> users (user_id));
+joinable!(follows -> crates (crate_id));
+joinable!(follows -> users (user_id));
+joinable!(version_authors -> users (user_id));
 joinable!(version_authors -> versions (version_id));
 joinable!(version_downloads -> versions (version_id));
-joinable!(crate_owners -> teams (owner_id));
-joinable!(crate_owners -> users (owner_id));
-joinable!(crate_owner_invitations -> crates (crate_id));
-joinable!(tokens -> emails (email_id));
+joinable!(versions -> crates (crate_id));

src/tests/user.rs

@@ -4,8 +4,7 @@ use conduit::{Handler, Method};
 
 use cargo_registry::token::ApiToken;
 use cargo_registry::krate::EncodableCrate;
-use cargo_registry::user::{Email, EncodablePrivateUser, EncodablePublicUser, NewEmail, NewUser,
-                           Token, User};
+use cargo_registry::user::{Email, EncodablePrivateUser, EncodablePublicUser, NewUser, User};
 use cargo_registry::version::EncodableVersion;
 
 use diesel::prelude::*;
@@ -684,7 +683,7 @@ fn test_insert_into_email_table_with_email_change() {
 */
 #[test]
 fn test_confirm_user_email() {
-    use cargo_registry::schema::{emails, tokens};
+    use cargo_registry::schema::emails;
 
     #[derive(Deserialize)]
     struct R {
@@ -701,7 +700,7 @@ fn test_confirm_user_email() {
     let user = {
         let conn = app.diesel_database.get().unwrap();
         let user = NewUser {
-            email: Some("potato@example.com"),
+            email: Some("potato2@example.com"),
             ..::new_user("potato")
         };
 
@@ -712,15 +711,10 @@ fn test_confirm_user_email() {
 
     let email_token = {
         let conn = app.diesel_database.get().unwrap();
-        let email_info = emails::table
-            .filter(emails::user_id.eq(user.id))
-            .first::<Email>(&*conn)
-            .unwrap();
-        let token_info = tokens::table
-            .filter(tokens::email_id.eq(email_info.id))
-            .first::<Token>(&*conn)
-            .unwrap();
-        token_info.token
+        Email::belonging_to(&user)
+            .select(emails::token)
+            .first::<String>(&*conn)
+            .unwrap()
     };
 
     let mut response = ok_resp!(
@@ -733,7 +727,7 @@ fn test_confirm_user_email() {
 
     let mut response = ok_resp!(middle.call(req.with_path("/api/v1/me").with_method(Method::Get),));
     let r = ::json::<R>(&mut response);
-    assert_eq!(r.user.email.unwrap(), "potato@example.com");
+    assert_eq!(r.user.email.unwrap(), "potato2@example.com");
     assert_eq!(r.user.login, "potato");
     assert!(r.user.email_verified);
     assert!(r.user.email_verification_sent);
@@ -745,8 +739,9 @@ fn test_confirm_user_email() {
 */
 #[test]
 fn test_existing_user_email() {
-    use cargo_registry::schema::{emails, users};
-    use diesel::insert;
+    use cargo_registry::schema::emails;
+    use chrono::NaiveDateTime;
+    use diesel::update;
 
     #[derive(Deserialize)]
     struct R {
@@ -757,24 +752,17 @@ fn test_existing_user_email() {
     let mut req = ::req(app.clone(), Method::Get, "/me");
     {
         let conn = app.diesel_database.get().unwrap();
-        let user = ::new_user("potahto");
-
-        // Deliberately not using User::create_or_update since that
-        // will try to send a verification email; we want to simulate
-        // a user who already had an email before we added verification.
-        let user = insert(&user)
-            .into(users::table)
-            .get_result::<User>(&*conn)
-            .unwrap();
-
-        let email = NewEmail {
-            user_id: user.id,
-            email: "[email protected]",
-            verified: false,
+        let new_user = NewUser {
+            email: Some("[email protected]"),
+            ..::new_user("potahto")
         };
-
-        insert(&email).into(emails::table).execute(&*conn).unwrap();
-
+        let user = new_user.create_or_update(&conn).unwrap();
+        update(Email::belonging_to(&user))
+            // Users created before we added verification will have
+            // `NULL` in the `token_generated_at` column.
+            .set(emails::token_generated_at.eq(None::<NaiveDateTime>))
+            .execute(&*conn)
+            .unwrap();
         ::sign_in_as(&mut req, &user);
     }