Skip to content

Commit 1f1394f

Browse files
authored
Merge pull request #9966 from Turbo87/async-update-user
controllers/user/update: Migrate to `diesel-async` queries
2 parents 01625ba + b3e73fc commit 1f1394f

File tree

2 files changed

+77
-65
lines changed

2 files changed

+77
-65
lines changed

src/controllers/user/update.rs

Lines changed: 61 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,11 @@ use crate::auth::AuthCheck;
33
use crate::controllers::helpers::ok_true;
44
use crate::models::NewEmail;
55
use crate::schema::{emails, users};
6-
use crate::tasks::spawn_blocking;
76
use crate::util::diesel::prelude::*;
87
use crate::util::errors::{bad_request, server_error, AppResult};
98
use axum::extract::Path;
109
use axum::response::Response;
1110
use axum::Json;
12-
use diesel_async::async_connection_wrapper::AsyncConnectionWrapper;
1311
use http::request::Parts;
1412
use lettre::Address;
1513
use secrecy::{ExposeSecret, SecretString};
@@ -32,85 +30,83 @@ pub async fn update_user(
3230
req: Parts,
3331
Json(user_update): Json<UserUpdate>,
3432
) -> AppResult<Response> {
33+
use diesel_async::RunQueryDsl;
34+
3535
let mut conn = state.db_write().await?;
3636
let auth = AuthCheck::default().check(&req, &mut conn).await?;
37-
spawn_blocking(move || {
38-
use diesel::RunQueryDsl;
39-
40-
let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into();
4137

42-
let user = auth.user();
38+
let user = auth.user();
4339

44-
// need to check if current user matches user to be updated
45-
if user.id != param_user_id {
46-
return Err(bad_request("current user does not match requested user"));
47-
}
40+
// need to check if current user matches user to be updated
41+
if user.id != param_user_id {
42+
return Err(bad_request("current user does not match requested user"));
43+
}
4844

49-
if let Some(publish_notifications) = &user_update.user.publish_notifications {
50-
if user.publish_notifications != *publish_notifications {
51-
diesel::update(user)
52-
.set(users::publish_notifications.eq(*publish_notifications))
53-
.execute(conn)?;
45+
if let Some(publish_notifications) = &user_update.user.publish_notifications {
46+
if user.publish_notifications != *publish_notifications {
47+
diesel::update(user)
48+
.set(users::publish_notifications.eq(*publish_notifications))
49+
.execute(&mut conn)
50+
.await?;
5451

55-
if !publish_notifications {
56-
let email_address = user.verified_email(conn)?;
52+
if !publish_notifications {
53+
let email_address = user.async_verified_email(&mut conn).await?;
5754

58-
if let Some(email_address) = email_address {
59-
let email = PublishNotificationsUnsubscribeEmail {
60-
user_name: &user.gh_login,
61-
domain: &state.emails.domain,
62-
};
55+
if let Some(email_address) = email_address {
56+
let email = PublishNotificationsUnsubscribeEmail {
57+
user_name: &user.gh_login,
58+
domain: &state.emails.domain,
59+
};
6360

64-
if let Err(error) = state.emails.send(&email_address, email) {
65-
warn!("Failed to send publish notifications unsubscribe email to {email_address}: {error}");
66-
}
61+
if let Err(error) = state.emails.async_send(&email_address, email).await {
62+
warn!("Failed to send publish notifications unsubscribe email to {email_address}: {error}");
6763
}
6864
}
6965
}
7066
}
67+
}
7168

72-
if let Some(user_email) = &user_update.user.email {
73-
let user_email = user_email.trim();
74-
75-
if user_email.is_empty() {
76-
return Err(bad_request("empty email rejected"));
77-
}
69+
if let Some(user_email) = &user_update.user.email {
70+
let user_email = user_email.trim();
7871

79-
user_email
80-
.parse::<Address>()
81-
.map_err(|_| bad_request("invalid email address"))?;
82-
83-
let new_email = NewEmail {
84-
user_id: user.id,
85-
email: user_email,
86-
};
87-
88-
let token = diesel::insert_into(emails::table)
89-
.values(&new_email)
90-
.on_conflict(emails::user_id)
91-
.do_update()
92-
.set(&new_email)
93-
.returning(emails::token)
94-
.get_result::<String>(conn)
95-
.map(SecretString::from)
96-
.map_err(|_| server_error("Error in creating token"))?;
97-
98-
// This swallows any errors that occur while attempting to send the email. Some users have
99-
// an invalid email set in their GitHub profile, and we should let them sign in even though
100-
// we're trying to silently use their invalid address during signup and can't send them an
101-
// email. They'll then have to provide a valid email address.
102-
let email = UserConfirmEmail {
103-
user_name: &user.gh_login,
104-
domain: &state.emails.domain,
105-
token,
106-
};
107-
108-
let _ = state.emails.send(user_email, email);
72+
if user_email.is_empty() {
73+
return Err(bad_request("empty email rejected"));
10974
}
11075

111-
ok_true()
112-
})
113-
.await
76+
user_email
77+
.parse::<Address>()
78+
.map_err(|_| bad_request("invalid email address"))?;
79+
80+
let new_email = NewEmail {
81+
user_id: user.id,
82+
email: user_email,
83+
};
84+
85+
let token = diesel::insert_into(emails::table)
86+
.values(&new_email)
87+
.on_conflict(emails::user_id)
88+
.do_update()
89+
.set(&new_email)
90+
.returning(emails::token)
91+
.get_result::<String>(&mut conn)
92+
.await
93+
.map(SecretString::from)
94+
.map_err(|_| server_error("Error in creating token"))?;
95+
96+
// This swallows any errors that occur while attempting to send the email. Some users have
97+
// an invalid email set in their GitHub profile, and we should let them sign in even though
98+
// we're trying to silently use their invalid address during signup and can't send them an
99+
// email. They'll then have to provide a valid email address.
100+
let email = UserConfirmEmail {
101+
user_name: &user.gh_login,
102+
domain: &state.emails.domain,
103+
token,
104+
};
105+
106+
let _ = state.emails.async_send(user_email, email).await;
107+
}
108+
109+
ok_true()
114110
}
115111

116112
pub struct UserConfirmEmail<'a> {

src/models/user.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,22 @@ impl User {
104104
.optional()
105105
}
106106

107+
/// Queries the database for the verified emails
108+
/// belonging to a given user
109+
pub async fn async_verified_email(
110+
&self,
111+
conn: &mut AsyncPgConnection,
112+
) -> QueryResult<Option<String>> {
113+
use diesel_async::RunQueryDsl;
114+
115+
Email::belonging_to(self)
116+
.select(emails::email)
117+
.filter(emails::verified.eq(true))
118+
.first(conn)
119+
.await
120+
.optional()
121+
}
122+
107123
/// Queries for the email belonging to a particular user
108124
pub fn email(&self, conn: &mut impl Conn) -> QueryResult<Option<String>> {
109125
use diesel::RunQueryDsl;

0 commit comments

Comments
 (0)