Skip to content

Commit 462743a

Browse files
committed
Remove the tokens table
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.
1 parent 769c878 commit 462743a

File tree

7 files changed

+132
-194
lines changed

7 files changed

+132
-194
lines changed
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
DROP TRIGGER trigger_emails_reconfirm ON emails;
2+
DROP TRIGGER trigger_emails_set_token_generated_at ON emails;
3+
DROP FUNCTION reconfirm_email_on_email_change();
4+
DROP FUNCTION emails_set_token_generated_at();
5+
6+
CREATE TABLE tokens (
7+
id SERIAL PRIMARY KEY,
8+
email_id INTEGER NOT NULL UNIQUE REFERENCES emails (id),
9+
token TEXT NOT NULL,
10+
created_at TIMESTAMP NOT NULL DEFAULT NOW()
11+
);
12+
13+
INSERT INTO tokens (email_id, token, created_at)
14+
SELECT id, token, token_generated_at FROM emails WHERE token_generated_at IS NOT NULL;
15+
16+
ALTER TABLE emails DROP COLUMN token;
17+
ALTER TABLE emails DROP COLUMN token_generated_at;
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
ALTER TABLE emails ADD COLUMN token TEXT NOT NULL DEFAULT random_string(26);
2+
ALTER TABLE emails ADD COLUMN token_generated_at TIMESTAMP;
3+
UPDATE emails SET token = tokens.token, token_generated_at = tokens.created_at
4+
FROM tokens WHERE tokens.email_id = emails.id;
5+
DROP TABLE tokens;
6+
7+
CREATE FUNCTION emails_set_token_generated_at() RETURNS trigger AS $$
8+
BEGIN
9+
NEW.token_generated_at := CURRENT_TIMESTAMP;
10+
RETURN NEW;
11+
END
12+
$$ LANGUAGE plpgsql;
13+
14+
CREATE FUNCTION reconfirm_email_on_email_change() RETURNS trigger AS $$
15+
BEGIN
16+
IF NEW.email IS DISTINCT FROM OLD.email THEN
17+
NEW.token := random_string(26);
18+
NEW.verified := false;
19+
END IF;
20+
RETURN NEW;
21+
END
22+
$$ LANGUAGE plpgsql;
23+
24+
CREATE TRIGGER trigger_emails_set_token_generated_at BEFORE
25+
INSERT OR UPDATE OF token ON emails
26+
FOR EACH ROW EXECUTE PROCEDURE emails_set_token_generated_at();
27+
28+
CREATE TRIGGER trigger_emails_reconfirm BEFORE UPDATE
29+
ON emails
30+
FOR EACH ROW EXECUTE PROCEDURE reconfirm_email_on_email_change();
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE emails DROP CONSTRAINT fk_emails_user_id;
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE emails ADD CONSTRAINT fk_emails_user_id FOREIGN KEY (user_id) REFERENCES users (id);

src/schema.rs

Lines changed: 22 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,18 @@ table! {
437437
///
438438
/// (Automatically generated by Diesel.)
439439
verified -> Bool,
440+
/// The `token` column of the `emails` table.
441+
///
442+
/// Its SQL type is `Text`.
443+
///
444+
/// (Automatically generated by Diesel.)
445+
token -> Text,
446+
/// The `token_generated_at` column of the `emails` table.
447+
///
448+
/// Its SQL type is `Nullable<Timestamp>`.
449+
///
450+
/// (Automatically generated by Diesel.)
451+
token_generated_at -> Nullable<Timestamp>,
440452
}
441453
}
442454

@@ -558,38 +570,6 @@ table! {
558570
}
559571
}
560572

561-
table! {
562-
/// Representation of the `tokens` table.
563-
///
564-
/// (Automatically generated by Diesel.)
565-
tokens (id) {
566-
/// The `id` column of the `tokens` table.
567-
///
568-
/// Its SQL type is `Int4`.
569-
///
570-
/// (Automatically generated by Diesel.)
571-
id -> Int4,
572-
/// The `email_id` column of the `tokens` table.
573-
///
574-
/// Its SQL type is `Int4`.
575-
///
576-
/// (Automatically generated by Diesel.)
577-
email_id -> Int4,
578-
/// The `token` column of the `tokens` table.
579-
///
580-
/// Its SQL type is `Varchar`.
581-
///
582-
/// (Automatically generated by Diesel.)
583-
token -> Varchar,
584-
/// The `created_at` column of the `tokens` table.
585-
///
586-
/// Its SQL type is `Timestamp`.
587-
///
588-
/// (Automatically generated by Diesel.)
589-
created_at -> Timestamp,
590-
}
591-
}
592-
593573
table! {
594574
/// Representation of the `users` table.
595575
///
@@ -784,22 +764,22 @@ table! {
784764
}
785765
}
786766

787-
joinable!(follows -> users (user_id));
788-
joinable!(version_authors -> users (user_id));
789-
joinable!(crates_keywords -> keywords (keyword_id));
790-
joinable!(crates_categories -> categories (category_id));
791767
joinable!(api_tokens -> users (user_id));
792768
joinable!(crate_downloads -> crates (crate_id));
769+
joinable!(crate_owner_invitations -> crates (crate_id));
793770
joinable!(crate_owners -> crates (crate_id));
771+
joinable!(crate_owners -> teams (owner_id));
772+
joinable!(crate_owners -> users (owner_id));
773+
joinable!(crates_categories -> categories (category_id));
794774
joinable!(crates_categories -> crates (crate_id));
795775
joinable!(crates_keywords -> crates (crate_id));
776+
joinable!(crates_keywords -> keywords (keyword_id));
796777
joinable!(dependencies -> crates (crate_id));
797-
joinable!(follows -> crates (crate_id));
798-
joinable!(versions -> crates (crate_id));
799778
joinable!(dependencies -> versions (version_id));
779+
joinable!(emails -> users (user_id));
780+
joinable!(follows -> crates (crate_id));
781+
joinable!(follows -> users (user_id));
782+
joinable!(version_authors -> users (user_id));
800783
joinable!(version_authors -> versions (version_id));
801784
joinable!(version_downloads -> versions (version_id));
802-
joinable!(crate_owners -> teams (owner_id));
803-
joinable!(crate_owners -> users (owner_id));
804-
joinable!(crate_owner_invitations -> crates (crate_id));
805-
joinable!(tokens -> emails (email_id));
785+
joinable!(versions -> crates (crate_id));

src/tests/user.rs

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ use conduit::{Handler, Method};
44

55
use cargo_registry::token::ApiToken;
66
use cargo_registry::krate::EncodableCrate;
7-
use cargo_registry::user::{Email, EncodablePrivateUser, EncodablePublicUser, NewEmail, NewUser,
8-
Token, User};
7+
use cargo_registry::user::{Email, EncodablePrivateUser, EncodablePublicUser, NewUser, User};
98
use cargo_registry::version::EncodableVersion;
109

1110
use diesel::prelude::*;
@@ -641,7 +640,7 @@ fn test_insert_into_email_table_with_email_change() {
641640
*/
642641
#[test]
643642
fn test_confirm_user_email() {
644-
use cargo_registry::schema::{emails, tokens};
643+
use cargo_registry::schema::emails;
645644

646645
#[derive(Deserialize)]
647646
struct R {
@@ -669,15 +668,10 @@ fn test_confirm_user_email() {
669668

670669
let email_token = {
671670
let conn = app.diesel_database.get().unwrap();
672-
let email_info = emails::table
673-
.filter(emails::user_id.eq(user.id))
674-
.first::<Email>(&*conn)
675-
.unwrap();
676-
let token_info = tokens::table
677-
.filter(tokens::email_id.eq(email_info.id))
678-
.first::<Token>(&*conn)
679-
.unwrap();
680-
token_info.token
671+
Email::belonging_to(&user)
672+
.select(emails::token)
673+
.first::<String>(&*conn)
674+
.unwrap()
681675
};
682676

683677
let mut response = ok_resp!(
@@ -702,8 +696,9 @@ fn test_confirm_user_email() {
702696
*/
703697
#[test]
704698
fn test_existing_user_email() {
705-
use cargo_registry::schema::{emails, users};
706-
use diesel::insert;
699+
use cargo_registry::schema::emails;
700+
use diesel::update;
701+
use time::Timespec;
707702

708703
#[derive(Deserialize)]
709704
struct R {
@@ -714,24 +709,17 @@ fn test_existing_user_email() {
714709
let mut req = ::req(app.clone(), Method::Get, "/me");
715710
{
716711
let conn = app.diesel_database.get().unwrap();
717-
let user = ::new_user("potahto");
718-
719-
// Deliberately not using User::create_or_update since that
720-
// will try to send a verification email; we want to simulate
721-
// a user who already had an email before we added verification.
722-
let user = insert(&user)
723-
.into(users::table)
724-
.get_result::<User>(&*conn)
725-
.unwrap();
726-
727-
let email = NewEmail {
728-
user_id: user.id,
729-
730-
verified: false,
712+
let new_user = NewUser {
713+
email: Some("[email protected]"),
714+
..::new_user("potahto")
731715
};
732-
733-
insert(&email).into(emails::table).execute(&*conn).unwrap();
734-
716+
let user = new_user.create_or_update(&conn).unwrap();
717+
update(Email::belonging_to(&user))
718+
// Users created before we added verification will have
719+
// `NULL` in the `token_generated_at` column.
720+
.set(emails::token_generated_at.eq(None::<Timespec>))
721+
.execute(&*conn)
722+
.unwrap();
735723
::sign_in_as(&mut req, &user);
736724
}
737725

0 commit comments

Comments
 (0)