diff --git a/Cargo.lock b/Cargo.lock index 2d27c5e3a4d..376d8a32b23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2831,11 +2831,14 @@ version = "0.11.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0161e452348e399deb685ba05e55ee116cae9410f4f51fe42d597361444521d9" dependencies = [ + "async-trait", "base64 0.22.1", "chumsky", "email-encoding", "email_address", "fastrand", + "futures-io", + "futures-util", "hostname", "httpdate", "idna", @@ -2846,6 +2849,7 @@ dependencies = [ "quoted_printable", "socket2", "tokio", + "tokio-native-tls", "url", "uuid", ] diff --git a/Cargo.toml b/Cargo.toml index db5c29dcb05..da5aaf9903b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,7 +84,7 @@ indicatif = "=0.17.9" ipnetwork = "=0.20.0" json-subscriber = "=0.2.2" tikv-jemallocator = { version = "=0.6.0", features = ['unprefixed_malloc_on_supported_platforms', 'profiling'] } -lettre = { version = "=0.11.10", default-features = false, features = ["file-transport", "smtp-transport", "native-tls", "hostname", "builder"] } +lettre = { version = "=0.11.10", default-features = false, features = ["file-transport", "smtp-transport", "hostname", "builder", "tokio1", "tokio1-native-tls"] } minijinja = "=2.5.0" mockall = "=0.13.0" native-tls = "=0.2.12" diff --git a/src/controllers/user/resend.rs b/src/controllers/user/resend.rs index 06afde01238..bcc65edab10 100644 --- a/src/controllers/user/resend.rs +++ b/src/controllers/user/resend.rs @@ -3,7 +3,6 @@ use crate::app::AppState; use crate::auth::AuthCheck; use crate::controllers::helpers::ok_true; use crate::models::Email; -use crate::tasks::spawn_blocking; use crate::util::errors::AppResult; use crate::util::errors::{bad_request, BoxedAppError}; use axum::extract::Path; @@ -38,19 +37,17 @@ pub async fn regenerate_token_and_send( .optional()? .ok_or_else(|| bad_request("Email could not be found"))?; - spawn_blocking(move || { - let email1 = UserConfirmEmail { - user_name: &auth.user().gh_login, - domain: &state.emails.domain, - token: email.token, - }; - - state - .emails - .send(&email.email, email1) - .map_err(BoxedAppError::from) - }) - .await + let email1 = UserConfirmEmail { + user_name: &auth.user().gh_login, + domain: &state.emails.domain, + token: email.token, + }; + + state + .emails + .async_send(&email.email, email1) + .await + .map_err(BoxedAppError::from) } .scope_boxed() }) @@ -74,7 +71,7 @@ mod tests { assert_eq!(response.status(), StatusCode::FORBIDDEN); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"this action requires authentication"}]}"#); - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); } #[tokio::test(flavor = "multi_thread")] @@ -87,7 +84,7 @@ mod tests { assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"current user does not match requested user"}]}"#); - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); } #[tokio::test(flavor = "multi_thread")] @@ -99,6 +96,6 @@ mod tests { assert_eq!(response.status(), StatusCode::OK); assert_snapshot!(response.text(), @r#"{"ok":true}"#); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } } diff --git a/src/email.rs b/src/email.rs index bb5e21fcd05..79312d7567e 100644 --- a/src/email.rs +++ b/src/email.rs @@ -3,12 +3,14 @@ use crate::Env; use lettre::address::Envelope; use lettre::message::header::ContentType; use lettre::message::Mailbox; -use lettre::transport::file::FileTransport; +use lettre::transport::file::AsyncFileTransport; use lettre::transport::smtp::authentication::{Credentials, Mechanism}; -use lettre::transport::smtp::SmtpTransport; -use lettre::transport::stub::StubTransport; -use lettre::{Address, Message, Transport}; +use lettre::transport::smtp::AsyncSmtpTransport; +use lettre::transport::stub::AsyncStubTransport; +use lettre::{Address, AsyncTransport, Message, Tokio1Executor}; use rand::distributions::{Alphanumeric, DistString}; +use std::sync::Arc; +use tokio::runtime::Handle; pub trait Email { fn subject(&self) -> String; @@ -36,7 +38,7 @@ impl Emails { let backend = match (login, password, server) { (Ok(login), Ok(password), Ok(server)) => { - let transport = SmtpTransport::relay(&server) + let transport = AsyncSmtpTransport::::relay(&server) .unwrap() .credentials(Credentials::new(login, password)) .authentication(vec![Mechanism::Plain]) @@ -45,8 +47,8 @@ impl Emails { EmailBackend::Smtp(Box::new(transport)) } _ => { - let transport = FileTransport::new("/tmp"); - EmailBackend::FileSystem(transport) + let transport = AsyncFileTransport::new("/tmp"); + EmailBackend::FileSystem(Arc::new(transport)) } }; @@ -67,7 +69,7 @@ impl Emails { /// to later assert the mails were sent. pub fn new_in_memory() -> Self { Self { - backend: EmailBackend::Memory(StubTransport::new_ok()), + backend: EmailBackend::Memory(AsyncStubTransport::new_ok()), domain: "crates.io".into(), from: DEFAULT_FROM.parse().unwrap(), } @@ -75,15 +77,15 @@ impl Emails { /// This is supposed to be used only during tests, to retrieve the messages stored in the /// "memory" backend. It's not cfg'd away because our integration tests need to access this. - pub fn mails_in_memory(&self) -> Option> { + pub async fn mails_in_memory(&self) -> Option> { if let EmailBackend::Memory(transport) = &self.backend { - Some(transport.messages()) + Some(transport.messages().await) } else { None } } - pub fn send(&self, recipient: &str, email: E) -> Result<(), EmailError> { + fn build_message(&self, recipient: &str, email: E) -> Result { // The message ID is normally generated by the SMTP server, but if we let it generate the // ID there will be no way for the crates.io application to know the ID of the message it // just sent, as it's not included in the SMTP response. @@ -102,7 +104,7 @@ impl Emails { let subject = email.subject(); let body = email.body(); - let email = Message::builder() + let message = Message::builder() .message_id(Some(message_id.clone())) .to(recipient.parse()?) .from(from) @@ -110,7 +112,24 @@ impl Emails { .header(ContentType::TEXT_PLAIN) .body(body)?; - self.backend.send(email).map_err(EmailError::TransportError) + Ok(message) + } + + pub fn send(&self, recipient: &str, email: E) -> Result<(), EmailError> { + let email = self.build_message(recipient, email)?; + + Handle::current() + .block_on(self.backend.send(email)) + .map_err(EmailError::TransportError) + } + + pub async fn async_send(&self, recipient: &str, email: E) -> Result<(), EmailError> { + let email = self.build_message(recipient, email)?; + + self.backend + .send(email) + .await + .map_err(EmailError::TransportError) } } @@ -129,19 +148,19 @@ enum EmailBackend { /// Backend used in production to send mails using SMTP. /// /// This is using `Box` to avoid a large size difference between variants. - Smtp(Box), + Smtp(Box>), /// Backend used locally during development, will store the emails in the provided directory. - FileSystem(FileTransport), + FileSystem(Arc>), /// Backend used during tests, will keep messages in memory to allow tests to retrieve them. - Memory(StubTransport), + Memory(AsyncStubTransport), } impl EmailBackend { - fn send(&self, message: Message) -> anyhow::Result<()> { + async fn send(&self, message: Message) -> anyhow::Result<()> { match self { - EmailBackend::Smtp(transport) => transport.send(&message).map(|_| ())?, - EmailBackend::FileSystem(transport) => transport.send(&message).map(|_| ())?, - EmailBackend::Memory(transport) => transport.send(&message).map(|_| ())?, + EmailBackend::Smtp(transport) => transport.send(message).await.map(|_| ())?, + EmailBackend::FileSystem(transport) => transport.send(message).await.map(|_| ())?, + EmailBackend::Memory(transport) => transport.send(message).await.map(|_| ())?, } Ok(()) @@ -171,20 +190,19 @@ mod tests { } } - #[test] - fn sending_to_invalid_email_fails() { + #[tokio::test] + async fn sending_to_invalid_email_fails() { let emails = Emails::new_in_memory(); - assert_err!(emails.send( - "String.Format(\"{0}.{1}@live.com\", FirstName, LastName)", - TestEmail - )); + let address = "String.Format(\"{0}.{1}@live.com\", FirstName, LastName)"; + assert_err!(emails.async_send(address, TestEmail).await); } - #[test] - fn sending_to_valid_email_succeeds() { + #[tokio::test] + async fn sending_to_valid_email_succeeds() { let emails = Emails::new_in_memory(); - assert_ok!(emails.send("someone@example.com", TestEmail)); + let address = "someone@example.com"; + assert_ok!(emails.async_send(address, TestEmail).await); } } diff --git a/src/tests/github_secret_scanning.rs b/src/tests/github_secret_scanning.rs index b6c9eeb64ac..0556460e647 100644 --- a/src/tests/github_secret_scanning.rs +++ b/src/tests/github_secret_scanning.rs @@ -45,7 +45,7 @@ async fn github_secret_alert_revokes_token() { let mut conn = app.async_db_conn().await; // Ensure no emails were sent up to this point - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); // Ensure that the token currently exists in the database let tokens: Vec = assert_ok!( @@ -94,7 +94,7 @@ async fn github_secret_alert_revokes_token() { assert_that!(tokens, len(eq(1))); // Ensure exactly one email was sent - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } #[tokio::test(flavor = "multi_thread")] @@ -103,7 +103,7 @@ async fn github_secret_alert_for_revoked_token() { let mut conn = app.async_db_conn().await; // Ensure no emails were sent up to this point - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); // Ensure that the token currently exists in the database let tokens: Vec = assert_ok!( @@ -155,7 +155,7 @@ async fn github_secret_alert_for_revoked_token() { assert_that!(tokens, len(eq(1))); // Ensure still no emails were sent - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); } #[tokio::test(flavor = "multi_thread")] @@ -164,7 +164,7 @@ async fn github_secret_alert_for_unknown_token() { let mut conn = app.async_db_conn().await; // Ensure no emails were sent up to this point - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); // Ensure that the token currently exists in the database let tokens: Vec = assert_ok!( @@ -197,7 +197,7 @@ async fn github_secret_alert_for_unknown_token() { assert_eq!(tokens[0].name, token.as_model().name); // Ensure still no emails were sent - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); } #[tokio::test(flavor = "multi_thread")] diff --git a/src/tests/krate/publish/auth.rs b/src/tests/krate/publish/auth.rs index a12a609fae8..706c619414e 100644 --- a/src/tests/krate/publish/auth.rs +++ b/src/tests/krate/publish/auth.rs @@ -30,7 +30,7 @@ async fn new_wrong_token() { assert_eq!(response.status(), StatusCode::FORBIDDEN); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"authentication failed"}]}"#); assert_that!(app.stored_files().await, empty()); - assert_that!(app.emails(), empty()); + assert_that!(app.emails().await, empty()); } #[tokio::test(flavor = "multi_thread")] @@ -50,5 +50,5 @@ async fn new_krate_wrong_user() { assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"this crate exists but you don't seem to be an owner. If you believe this is a mistake, perhaps you need to accept an invitation to be an owner before publishing."}]}"#); assert_that!(app.stored_files().await, empty()); - assert_that!(app.emails(), empty()); + assert_that!(app.emails().await, empty()); } diff --git a/src/tests/krate/publish/basics.rs b/src/tests/krate/publish/basics.rs index f666318abe2..8e753c87704 100644 --- a/src/tests/krate/publish/basics.rs +++ b/src/tests/krate/publish/basics.rs @@ -38,7 +38,7 @@ async fn new_krate() { .unwrap(); assert_eq!(email, "foo@example.com"); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } #[tokio::test(flavor = "multi_thread")] diff --git a/src/tests/krate/publish/emails.rs b/src/tests/krate/publish/emails.rs index b0503865a58..93b02e9a875 100644 --- a/src/tests/krate/publish/emails.rs +++ b/src/tests/krate/publish/emails.rs @@ -21,7 +21,7 @@ async fn new_krate_without_any_email_fails() { assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"A verified email address is required to publish crates to crates.io. Visit https://crates.io/settings/profile to set and verify your email address."}]}"#); assert_that!(app.stored_files().await, empty()); - assert_that!(app.emails(), empty()); + assert_that!(app.emails().await, empty()); } #[tokio::test(flavor = "multi_thread")] @@ -41,5 +41,5 @@ async fn new_krate_with_unverified_email_fails() { assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"A verified email address is required to publish crates to crates.io. Visit https://crates.io/settings/profile to set and verify your email address."}]}"#); assert_that!(app.stored_files().await, empty()); - assert_that!(app.emails(), empty()); + assert_that!(app.emails().await, empty()); } diff --git a/src/tests/owners.rs b/src/tests/owners.rs index f7af5023657..1f1e16446e5 100644 --- a/src/tests/owners.rs +++ b/src/tests/owners.rs @@ -137,7 +137,7 @@ async fn new_crate_owner() { let user2 = app.db_new_user("Bar"); token.add_named_owner("foo_owner", "BAR").await.good(); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); // accept invitation for user to be added as owner let krate: Crate = Crate::by_name("foo_owner").first(&mut conn).unwrap(); @@ -159,7 +159,7 @@ async fn new_crate_owner() { .await .good(); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } async fn create_and_add_owner( @@ -221,7 +221,7 @@ async fn modify_multiple_owners() { let user2 = create_and_add_owner(&app, &token, "user2", &krate).await; let user3 = create_and_add_owner(&app, &token, "user3", &krate).await; - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); // Deleting all owners is not allowed. let response = token @@ -254,7 +254,7 @@ async fn modify_multiple_owners() { assert_eq!(response.status(), StatusCode::OK); assert_snapshot!(response.text(), @r#"{"msg":"user user2 has been invited to be an owner of crate owners_multiple,user user3 has been invited to be an owner of crate owners_multiple","ok":true}"#); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); user2 .accept_ownership_invitation(&krate.name, krate.id) @@ -569,7 +569,7 @@ async fn test_accept_invitation_by_mail() { .good(); // Retrieve the ownership invitation - let invite_token = extract_token_from_invite_email(&app.emails()); + let invite_token = extract_token_from_invite_email(&app.emails().await); // Accept the invitation anonymously with a token anon.accept_ownership_invitation_by_token(&invite_token) @@ -688,7 +688,7 @@ async fn test_accept_expired_invitation_by_mail() { expire_invitation(&app, krate.id); // Retrieve the ownership invitation - let invite_token = extract_token_from_invite_email(&app.emails()); + let invite_token = extract_token_from_invite_email(&app.emails().await); // Try to accept the invitation, and ensure it fails. let resp = anon diff --git a/src/tests/routes/crates/owners/add.rs b/src/tests/routes/crates/owners/add.rs index 7de68640355..08959249dc2 100644 --- a/src/tests/routes/crates/owners/add.rs +++ b/src/tests/routes/crates/owners/add.rs @@ -204,7 +204,7 @@ async fn invite_already_invited_user() { CrateBuilder::new("crate_name", owner.as_model().user_id).expect_build(&mut conn); // Ensure no emails were sent up to this point - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); // Invite the user the first time let response = owner.add_named_owner("crate_name", "invited_user").await; @@ -212,7 +212,7 @@ async fn invite_already_invited_user() { assert_snapshot!(response.text(), @r#"{"msg":"user invited_user has been invited to be an owner of crate crate_name","ok":true}"#); // Check one email was sent, this will be the ownership invite email - assert_eq!(app.emails().len(), 1); + assert_eq!(app.emails().await.len(), 1); // Then invite the user a second time, the message should point out the user is already invited let response = owner.add_named_owner("crate_name", "invited_user").await; @@ -220,7 +220,7 @@ async fn invite_already_invited_user() { assert_snapshot!(response.text(), @r#"{"msg":"user invited_user already has a pending invitation to be an owner of crate crate_name","ok":true}"#); // Check that no new email is sent after the second invitation - assert_eq!(app.emails().len(), 1); + assert_eq!(app.emails().await.len(), 1); } #[tokio::test(flavor = "multi_thread")] @@ -232,7 +232,7 @@ async fn invite_with_existing_expired_invite() { let krate = CrateBuilder::new("crate_name", owner.as_model().user_id).expect_build(&mut conn); // Ensure no emails were sent up to this point - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); // Invite the user the first time let response = owner.add_named_owner("crate_name", "invited_user").await; @@ -240,7 +240,7 @@ async fn invite_with_existing_expired_invite() { assert_snapshot!(response.text(), @r#"{"msg":"user invited_user has been invited to be an owner of crate crate_name","ok":true}"#); // Check one email was sent, this will be the ownership invite email - assert_eq!(app.emails().len(), 1); + assert_eq!(app.emails().await.len(), 1); // Simulate the previous invite expiring expire_invitation(&app, krate.id); @@ -251,7 +251,7 @@ async fn invite_with_existing_expired_invite() { assert_snapshot!(response.text(), @r#"{"msg":"user invited_user has been invited to be an owner of crate crate_name","ok":true}"#); // Check that the email for the second invite was sent - assert_eq!(app.emails().len(), 2); + assert_eq!(app.emails().await.len(), 2); } #[tokio::test(flavor = "multi_thread")] @@ -335,7 +335,7 @@ async fn no_invite_emails_for_txn_rollback() { assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"could not find user with login `bananas`"}]}"#); // No emails should have been sent. - assert_eq!(app.emails().len(), 0); + assert_eq!(app.emails().await.len(), 0); // Remove the bad username. let _ = usernames.pop(); @@ -344,5 +344,5 @@ async fn no_invite_emails_for_txn_rollback() { assert_eq!(response.status(), StatusCode::OK); // 9 emails to the good invitees should have been sent. - assert_eq!(app.emails().len(), 9); + assert_eq!(app.emails().await.len(), 9); } diff --git a/src/tests/routes/me/tokens/create.rs b/src/tests/routes/me/tokens/create.rs index 1462f06119f..80f51ec905e 100644 --- a/src/tests/routes/me/tokens/create.rs +++ b/src/tests/routes/me/tokens/create.rs @@ -26,7 +26,7 @@ async fn create_token_invalid_request() { let response = user.put::<()>("/api/v1/me/tokens", invalid).await; assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"Failed to deserialize the JSON body into the target type: missing field `api_token` at line 1 column 14"}]}"#); - assert!(app.emails().is_empty()); + assert!(app.emails().await.is_empty()); } #[tokio::test(flavor = "multi_thread")] @@ -36,7 +36,7 @@ async fn create_token_no_name() { let response = user.put::<()>("/api/v1/me/tokens", empty_name).await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"name must have a value"}]}"#); - assert!(app.emails().is_empty()); + assert!(app.emails().await.is_empty()); } #[tokio::test(flavor = "multi_thread")] @@ -52,7 +52,7 @@ async fn create_token_exceeded_tokens_per_user() { let response = user.put::<()>("/api/v1/me/tokens", NEW_BAR).await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"maximum tokens per user is: 500"}]}"#); - assert!(app.emails().is_empty()); + assert!(app.emails().await.is_empty()); } #[tokio::test(flavor = "multi_thread")] @@ -83,7 +83,7 @@ async fn create_token_success() { assert_eq!(tokens[0].crate_scopes, None); assert_eq!(tokens[0].endpoint_scopes, None); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } #[tokio::test(flavor = "multi_thread")] @@ -118,7 +118,7 @@ async fn cannot_create_token_with_token() { .await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"cannot use an API token to create a new API token"}]}"#); - assert!(app.emails().is_empty()); + assert!(app.emails().await.is_empty()); } #[tokio::test(flavor = "multi_thread")] @@ -168,7 +168,7 @@ async fn create_token_with_scopes() { Some(vec![EndpointScope::PublishUpdate]) ); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } #[tokio::test(flavor = "multi_thread")] @@ -209,7 +209,7 @@ async fn create_token_with_null_scopes() { assert_eq!(tokens[0].crate_scopes, None); assert_eq!(tokens[0].endpoint_scopes, None); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } #[tokio::test(flavor = "multi_thread")] @@ -229,7 +229,7 @@ async fn create_token_with_empty_crate_scope() { .await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid crate scope"}]}"#); - assert!(app.emails().is_empty()); + assert!(app.emails().await.is_empty()); } #[tokio::test(flavor = "multi_thread")] @@ -249,7 +249,7 @@ async fn create_token_with_invalid_endpoint_scope() { .await; assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!(response.text(), @r#"{"errors":[{"detail":"invalid endpoint scope"}]}"#); - assert!(app.emails().is_empty()); + assert!(app.emails().await.is_empty()); } #[tokio::test(flavor = "multi_thread")] @@ -276,5 +276,5 @@ async fn create_token_with_expiry_date() { ".api_token.token" => insta::api_token_redaction(), }); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } diff --git a/src/tests/routes/users/update/publish_notifications.rs b/src/tests/routes/users/update/publish_notifications.rs index 5d050bcdf35..b4b60affa4e 100644 --- a/src/tests/routes/users/update/publish_notifications.rs +++ b/src/tests/routes/users/update/publish_notifications.rs @@ -16,7 +16,7 @@ async fn test_unsubscribe_and_resubscribe() { assert_eq!(response.status(), StatusCode::OK); // Assert that the user gets an initial publish email - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); // Unsubscribe from publish notifications let payload = json!({"user": { "publish_notifications": false }}); @@ -25,7 +25,7 @@ async fn test_unsubscribe_and_resubscribe() { assert_snapshot!(response.text(), @r#"{"ok":true}"#); // Assert that the user gets an unsubscribe email - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); // Publish the same crate again to check that the user doesn't get a publish email let pb = PublishBuilder::new("foo", "1.1.0"); @@ -33,7 +33,7 @@ async fn test_unsubscribe_and_resubscribe() { assert_eq!(response.status(), StatusCode::OK); // Assert that the user did not get a publish email this time - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); // Resubscribe to publish notifications let payload = json!({"user": { "publish_notifications": true }}); @@ -47,5 +47,5 @@ async fn test_unsubscribe_and_resubscribe() { assert_eq!(response.status(), StatusCode::OK); // Assert that the user got a publish email again - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } diff --git a/src/tests/team.rs b/src/tests/team.rs index 5a54002e1fc..71323591bf0 100644 --- a/src/tests/team.rs +++ b/src/tests/team.rs @@ -348,7 +348,7 @@ async fn publish_owned() { .await .good(); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); } /// Test trying to change owners (when only on an owning team) diff --git a/src/tests/user.rs b/src/tests/user.rs index 6bb3b459e8d..c0594b210ff 100644 --- a/src/tests/user.rs +++ b/src/tests/user.rs @@ -1,4 +1,5 @@ use crate::models::{ApiToken, Email, NewUser, User}; +use crate::tasks::spawn_blocking; use crate::tests::{ new_user, util::{MockCookieUser, RequestHelper}, @@ -112,15 +113,21 @@ async fn github_with_email_does_not_overwrite_email() { // Simulate logging in to crates.io after changing your email in GitHub + let emails = app.as_inner().emails.clone(); + let u = NewUser { // Use the same github ID to link to the existing account gh_id: model.gh_id, // the rest of the fields are arbitrary ..new_user("arbitrary_username") }; - let u = u - .create_or_update(Some(new_github_email), &app.as_inner().emails, &mut conn) - .unwrap(); + let u = spawn_blocking(move || { + let u = u.create_or_update(Some(new_github_email), &emails, &mut conn)?; + Ok::<_, anyhow::Error>(u) + }) + .await + .unwrap(); + let user_with_different_email_in_github = MockCookieUser::new(&app, u); let json = user_with_different_email_in_github.show_me().await; @@ -161,9 +168,14 @@ async fn test_confirm_user_email() { // email directly into the database and we want to test the verification flow here. let email = "potato2@example.com"; - let u = new_user("arbitrary_username") - .create_or_update(Some(email), &app.as_inner().emails, &mut conn) - .unwrap(); + let emails = app.as_inner().emails.clone(); + let (u, mut conn) = spawn_blocking(move || { + let u = new_user("arbitrary_username").create_or_update(Some(email), &emails, &mut conn)?; + Ok::<_, anyhow::Error>((u, conn)) + }) + .await + .unwrap(); + let user = MockCookieUser::new(&app, u); let user_model = user.as_model(); @@ -195,9 +207,15 @@ async fn test_existing_user_email() { // Simulate logging in via GitHub. Don't use app.db_new_user because it inserts a verified // email directly into the database and we want to test the verification flow here. let email = "potahto@example.com"; - let u = new_user("arbitrary_username") - .create_or_update(Some(email), &app.as_inner().emails, &mut conn) - .unwrap(); + + let emails = app.as_inner().emails.clone(); + let (u, mut conn) = spawn_blocking(move || { + let u = new_user("arbitrary_username").create_or_update(Some(email), &emails, &mut conn)?; + Ok::<_, anyhow::Error>((u, conn)) + }) + .await + .unwrap(); + update(Email::belonging_to(&u)) // Users created before we added verification will have // `NULL` in the `token_generated_at` column. diff --git a/src/tests/util/test_app.rs b/src/tests/util/test_app.rs index d0eff98107b..023c28f9b41 100644 --- a/src/tests/util/test_app.rs +++ b/src/tests/util/test_app.rs @@ -179,12 +179,12 @@ impl TestApp { .collect() } - pub fn emails(&self) -> Vec { - let emails = self.as_inner().emails.mails_in_memory().unwrap(); + pub async fn emails(&self) -> Vec { + let emails = self.as_inner().emails.mails_in_memory().await.unwrap(); emails.into_iter().map(|(_, email)| email).collect() } - pub fn emails_snapshot(&self) -> String { + pub async fn emails_snapshot(&self) -> String { static EMAIL_HEADER_REGEX: LazyLock = LazyLock::new(|| Regex::new(r"(Message-ID|Date): [^\r\n]+\r\n").unwrap()); @@ -200,6 +200,7 @@ impl TestApp { static SEPARATOR: &str = "\n----------------------------------------\n\n"; self.emails() + .await .into_iter() .map(|email| { let email = EMAIL_HEADER_REGEX.replace_all(&email, ""); diff --git a/src/tests/worker/sync_admins.rs b/src/tests/worker/sync_admins.rs index 4add33bd49b..7bb8d2cbd65 100644 --- a/src/tests/worker/sync_admins.rs +++ b/src/tests/worker/sync_admins.rs @@ -47,14 +47,14 @@ async fn test_sync_admins_job() { let expected_admins = vec![("existing-admin".into(), 1), ("new-admin".into(), 3)]; assert_eq!(admins, expected_admins); - assert_snapshot!(app.emails_snapshot()); + assert_snapshot!(app.emails_snapshot().await); // Run the job again to verify that no new emails are sent // for `new-admin-without-account`. SyncAdmins.async_enqueue(&mut conn).await.unwrap(); app.run_pending_background_jobs().await; - assert_eq!(app.emails().len(), 2); + assert_eq!(app.emails().await.len(), 2); } fn mock_permission(people: Vec) -> Permission { diff --git a/src/worker/jobs/expiry_notification.rs b/src/worker/jobs/expiry_notification.rs index d8188b1d796..bff89e39b1b 100644 --- a/src/worker/jobs/expiry_notification.rs +++ b/src/worker/jobs/expiry_notification.rs @@ -184,11 +184,14 @@ mod tests { let (_test_db, mut conn) = test_db_connection(); // Set up a user and a token that is about to expire. - let user = NewUser::new(0, "a", None, None, "token").create_or_update( - Some("testuser@test.com"), - &Emails::new_in_memory(), - &mut conn, - )?; + let (user, mut conn) = spawn_blocking(move || { + let user = NewUser::new(0, "a", None, None, "token"); + let emails = Emails::new_in_memory(); + let user = user.create_or_update(Some("testuser@test.com"), &emails, &mut conn)?; + Ok::<_, anyhow::Error>((user, conn)) + }) + .await?; + let token = PlainToken::generate(); let token: ApiToken = diesel::insert_into(api_tokens::table) @@ -217,10 +220,17 @@ mod tests { } // Check that the token is about to expire. - check(&emails, &mut conn)?; + let mut conn = spawn_blocking({ + let emails = emails.clone(); + move || { + check(&emails, &mut conn)?; + Ok::<_, anyhow::Error>(conn) + } + }) + .await?; // Check that an email was sent. - let sent_mail = emails.mails_in_memory().unwrap(); + let sent_mail = emails.mails_in_memory().await.unwrap(); assert_eq!(sent_mail.len(), 1); let sent = &sent_mail[0]; assert_eq!(&sent.0.to(), &["testuser@test.com".parse::
()?]); @@ -255,10 +265,14 @@ mod tests { .get_result(&mut conn)?; // Check that the token is not about to expire. - check(&emails, &mut conn)?; + spawn_blocking({ + let emails = emails.clone(); + move || check(&emails, &mut conn) + }) + .await?; // Check that no email was sent. - let sent_mail = emails.mails_in_memory().unwrap(); + let sent_mail = emails.mails_in_memory().await.unwrap(); assert_eq!(sent_mail.len(), 1); Ok(()) diff --git a/src/worker/jobs/sync_admins.rs b/src/worker/jobs/sync_admins.rs index 743ff1d672f..a59e915ea3f 100644 --- a/src/worker/jobs/sync_admins.rs +++ b/src/worker/jobs/sync_admins.rs @@ -1,6 +1,5 @@ use crate::email::Email; use crate::schema::{emails, users}; -use crate::tasks::spawn_blocking; use crate::worker::Environment; use crates_io_worker::BackgroundJob; use diesel::prelude::*; @@ -142,27 +141,22 @@ impl BackgroundJob for SyncAdmins { let email = AdminAccountEmail::new(added_admins, removed_admins); - spawn_blocking(move || { - for database_admin in &database_admins { - let (_, _, email_address) = database_admin; - if let Some(email_address) = email_address { - if let Err(error) = ctx.emails.send(email_address, email.clone()) { - warn!( - "Failed to send email to admin {} ({}, github_id: {}): {}", - database_admin.1, email_address, database_admin.0, error - ); - } - } else { + for database_admin in &database_admins { + let (_, _, email_address) = database_admin; + if let Some(email_address) = email_address { + if let Err(error) = ctx.emails.async_send(email_address, email.clone()).await { warn!( - "No email address found for admin {} (github_id: {})", - database_admin.1, database_admin.0 + "Failed to send email to admin {} ({}, github_id: {}): {}", + database_admin.1, email_address, database_admin.0, error ); } + } else { + warn!( + "No email address found for admin {} (github_id: {})", + database_admin.1, database_admin.0 + ); } - - Ok::<_, anyhow::Error>(()) - }) - .await?; + } Ok(()) } diff --git a/src/worker/jobs/typosquat.rs b/src/worker/jobs/typosquat.rs index 627afe7fef8..7294ef55b89 100644 --- a/src/worker/jobs/typosquat.rs +++ b/src/worker/jobs/typosquat.rs @@ -127,8 +127,8 @@ mod tests { use super::*; - #[test] - fn integration() -> anyhow::Result<()> { + #[tokio::test] + async fn integration() -> anyhow::Result<()> { let emails = Emails::new_in_memory(); let (_test_db, mut conn) = test_db_connection(); @@ -138,6 +138,7 @@ mod tests { // Prime the cache so it only includes the crate we just created. let cache = Cache::new(vec!["admin@example.com".to_string()], &mut conn)?; + let cache = Arc::new(cache); // Now we'll create new crates: one problematic, one not so. let other_user = faker::user(&mut conn, "b")?; @@ -157,12 +158,25 @@ mod tests { )?; // Run the check with a crate that shouldn't cause problems. - check(&emails, &cache, &mut conn, &angel.name)?; - assert!(emails.mails_in_memory().unwrap().is_empty()); + let mut conn = spawn_blocking({ + let emails = emails.clone(); + let cache = cache.clone(); + move || { + check(&emails, &cache, &mut conn, &angel.name)?; + Ok::<_, anyhow::Error>(conn) + } + }) + .await?; + assert!(emails.mails_in_memory().await.unwrap().is_empty()); // Now run the check with a less innocent crate. - check(&emails, &cache, &mut conn, &demon.name)?; - let sent_mail = emails.mails_in_memory().unwrap(); + spawn_blocking({ + let emails = emails.clone(); + let cache = cache.clone(); + move || check(&emails, &cache, &mut conn, &demon.name) + }) + .await?; + let sent_mail = emails.mails_in_memory().await.unwrap(); assert!(!sent_mail.is_empty()); let sent = sent_mail.into_iter().next().unwrap(); assert_eq!(&sent.0.to(), &["admin@example.com".parse::
()?]);