From a2bdf14a976f4f3b7e8d99e46bf7172cd8d21e49 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 20:39:01 -0700 Subject: [PATCH 01/17] ch7: [db migration] create new sub-token table Signed-off-by: Jin Dong --- ...331033331_make_status_not_null_in_subscriptions.sql | 10 ++++++++++ ...20240331033641_create_subscription_tokens_table.sql | 7 +++++++ 2 files changed, 17 insertions(+) create mode 100644 migrations/20240331033331_make_status_not_null_in_subscriptions.sql create mode 100644 migrations/20240331033641_create_subscription_tokens_table.sql diff --git a/migrations/20240331033331_make_status_not_null_in_subscriptions.sql b/migrations/20240331033331_make_status_not_null_in_subscriptions.sql new file mode 100644 index 0000000..a9553ed --- /dev/null +++ b/migrations/20240331033331_make_status_not_null_in_subscriptions.sql @@ -0,0 +1,10 @@ +-- Add migration script here +-- 1. backfill null status to `confirmed` in `subscriptions` table +-- 2. and then mark status column as NOT NULL +-- Make 1 and 2 in a single transaction +BEGIN; + UPDATE subscriptions + SET status = 'confirmed' + WHERE status IS NULL; + ALTER TABLE subscriptions ALTER COLUMN status SET NOT NULL; +COMMIT; \ No newline at end of file diff --git a/migrations/20240331033641_create_subscription_tokens_table.sql b/migrations/20240331033641_create_subscription_tokens_table.sql new file mode 100644 index 0000000..ee78d88 --- /dev/null +++ b/migrations/20240331033641_create_subscription_tokens_table.sql @@ -0,0 +1,7 @@ +-- Add migration script here +CREATE TABLE subscription_tokens ( + subscription_token TEXT NOT NULL, + subscriber_id uuid NOT NULL + REFERENCES subscriptions (id), + PRIMARY KEY (subscription_token) +); \ No newline at end of file From e67fe55a5632c78ec52e6b0788247d56c7dc45a3 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 20:47:20 -0700 Subject: [PATCH 02/17] ch7: add mock email server and email test Signed-off-by: Jin Dong --- tests/api/helpers.rs | 7 +++++++ tests/api/subscriptions.rs | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index 94c2881..afee64a 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -1,6 +1,7 @@ use once_cell::sync::Lazy; use sqlx::{Connection, Executor, PgConnection, PgPool}; use uuid::Uuid; +use wiremock::MockServer; use zero2prod::configuration::{get_configuration, DatabaseSettings}; use zero2prod::startup::{get_connection_pool, Application}; use zero2prod::telemetry::{get_subscriber, init_subscriber}; @@ -8,6 +9,7 @@ use zero2prod::telemetry::{get_subscriber, init_subscriber}; pub struct TestApp { pub address: String, pub db_pool: PgPool, + pub email_server: MockServer, } impl TestApp { @@ -41,12 +43,16 @@ pub async fn spawn_app() -> TestApp { // All other invocations will instead skip execution. Lazy::force(&TRACING); + let email_server = MockServer::start().await; + // Randomize configuration to ensure test isolation. let configuration = { let mut c = get_configuration().expect("Failed to read configuration"); // create a new database for each test, by using a random name c.database.database_name = Uuid::new_v4().to_string(); c.application.port = 0; + // same for the mock email server + c.email_client.base_url = email_server.uri(); c }; @@ -65,6 +71,7 @@ pub async fn spawn_app() -> TestApp { TestApp { address, db_pool: get_connection_pool(&configuration.database), + email_server, } } diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 561e4a4..cf9ea4a 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -1,5 +1,8 @@ use crate::helpers::spawn_app; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, ResponseTemplate}; + #[tokio::test] async fn subscribe_returns_200_for_valid_form_data() { let app = spawn_app().await; @@ -63,3 +66,19 @@ async fn subscribe_returns_a_400_when_fields_are_present_but_invalid() { ); } } + +#[tokio::test] +async fn subscribe_sends_a_confirmation_email_for_valid_data() { + let app = spawn_app().await; + let body = "name=le%20guin&email=ursula_le_guin%40gmail.com"; + + // The mock email server should receive an email in the `/email` endpoint. + Mock::given(path("/email")) + .and(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&app.email_server) + .await; + + app.post_subscriptions(body.into()).await; +} From af99cf661d7162972f79d67b66d055677b9d1f0f Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 20:59:58 -0700 Subject: [PATCH 03/17] ch7: send an email in subscription endpoint Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 32 ++++++++++++++++++++++++++------ tests/api/subscriptions.rs | 13 +++++++++++-- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 974aca2..56e64c4 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -3,7 +3,10 @@ use chrono::Utc; use sqlx::PgPool; use uuid::Uuid; -use crate::domain::{NewSubscriber, SubscriberEmail, SubscriberName}; +use crate::{ + domain::{NewSubscriber, SubscriberEmail, SubscriberName}, + email_client::EmailClient, +}; #[derive(serde::Deserialize)] pub struct FormData { @@ -24,13 +27,17 @@ impl TryFrom for NewSubscriber { #[tracing::instrument( name = "Adding a new subscriber", // span message, fn name by default - skip(form, db_pool), // skip these two fields in the span + skip(form, db_pool, email_client), // skip these two fields in the span fields( subscriber_email = %form.email, subscriber_name = %form.name ) )] -pub async fn subscribe(form: web::Form, db_pool: web::Data) -> impl Responder { +pub async fn subscribe( + form: web::Form, + db_pool: web::Data, + email_client: web::Data, +) -> impl Responder { // `web::Form` is a wrapper around `FormData` // `form.0` gives us access to the underlying `FormData` let new_subscriber = match form.0.try_into() { @@ -38,10 +45,23 @@ pub async fn subscribe(form: web::Form, db_pool: web::Data) -> Err(_) => return HttpResponse::BadRequest().finish(), }; - match insert_subscriber(&new_subscriber, &db_pool).await { - Ok(_) => HttpResponse::Ok().finish(), - Err(_) => HttpResponse::InternalServerError().finish(), + if insert_subscriber(&new_subscriber, &db_pool).await.is_err() { + HttpResponse::InternalServerError().finish(); } + + if email_client + .send_email( + new_subscriber.email, + "welcome", + "html_content", + "text_content", + ) + .await + .is_err() + { + return HttpResponse::InternalServerError().finish(); + } + HttpResponse::Ok().finish() } #[tracing::instrument( diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index cf9ea4a..0162b53 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -5,13 +5,22 @@ use wiremock::{Mock, ResponseTemplate}; #[tokio::test] async fn subscribe_returns_200_for_valid_form_data() { + // Arrange let app = spawn_app().await; - let body = "name=le%20guin&email=ursula_le_guin%40gmail.com"; + + // Mock + Mock::given(path("/email")) + .and(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .mount(&app.email_server) + .await; + + // Act let resp = app.post_subscriptions(body.into()).await; + // Assert assert_eq!(200, resp.status().as_u16()); - let saved = sqlx::query!("SELECT email, name FROM subscriptions",) .fetch_one(&app.db_pool) .await From 490c732eb66976204f3fcf3367258688c9d2ea62 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 21:12:24 -0700 Subject: [PATCH 04/17] ch7: [test] confirmation email has link Signed-off-by: Jin Dong --- Cargo.lock | 10 ++++++++++ Cargo.toml | 1 + tests/api/subscriptions.rs | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 56bcddc..483def1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1326,6 +1326,15 @@ version = "0.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0717cef1bc8b636c6e1c1bbdefc09e6322da8a9321966e8928ef80d20f7f770f" +[[package]] +name = "linkify" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1dfa36d52c581e9ec783a7ce2a5e0143da6237be5811a0b3153fedfdbe9f780" +dependencies = [ + "memchr", +] + [[package]] name = "linux-raw-sys" version = "0.4.13" @@ -3451,6 +3460,7 @@ dependencies = [ "claims", "config", "fake", + "linkify", "log", "once_cell", "quickcheck", diff --git a/Cargo.toml b/Cargo.toml index 60e18d1..4141cc4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,3 +54,4 @@ quickcheck = "0.9.2" quickcheck_macros = "0.9.1" wiremock = "0.6.0" serde_json = "1.0.115" +linkify = "0.10.0" diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 0162b53..9a8a456 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -91,3 +91,39 @@ async fn subscribe_sends_a_confirmation_email_for_valid_data() { app.post_subscriptions(body.into()).await; } + +#[tokio::test] +async fn subscribe_sends_a_confirmation_email_with_a_link() { + // Arrange + let app = spawn_app().await; + let body = "name=le%20guin&email=ursula_le_guin%40gmail.com"; + + Mock::given(path("/email")) + .and(method("POST")) + .respond_with(ResponseTemplate::new(200)) + // We are not setting an expectation here anymore + // The test is focused on another aspect of the app behavior. + // .expect(1) + .mount(&app.email_server) + .await; + + // Act + app.post_subscriptions(body.into()).await; + + // Assert + let email_request = &app.email_server.received_requests().await.unwrap()[0]; + let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); + let get_link = |s: &str| { + let links: Vec<_> = linkify::LinkFinder::new() + .links(s) + .filter(|l| *l.kind() == linkify::LinkKind::Url) + .collect(); + assert_eq!(links.len(), 1); + links[0].as_str().to_owned() + }; + + let html_link = get_link(&body["HtmlBody"].as_str().unwrap()); + let text_link = get_link(&body["TextBody"].as_str().unwrap()); + // The two links should be identical + assert_eq!(html_link, text_link); +} \ No newline at end of file From 116d4e7e515845babf0a5a69b9ae6247d804fa4b Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 21:15:11 -0700 Subject: [PATCH 05/17] ch7: subscriptions handler sends an email with valid link Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 56e64c4..a4b308c 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -49,12 +49,21 @@ pub async fn subscribe( HttpResponse::InternalServerError().finish(); } + + let confirmation_link = "https://there-is-no-such-domain.com/subscriptions/confirm"; if email_client .send_email( new_subscriber.email, "welcome", - "html_content", - "text_content", + &format!( + "Welcome to our newsletter!
\ + Click here to confirm your subscription.", + confirmation_link + ), + &format!( + "Welcome to our newsletter!\nVisit {} to confirm your subscription.", + confirmation_link + ), ) .await .is_err() From 0f34e2af063c2fa629b6e5c89fe7ad4ccbc44cd1 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 21:26:33 -0700 Subject: [PATCH 06/17] ch7: [refactor] move send_confirmation_email to a fn Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 41 ++++++++++++++++++++++--------------- tests/api/subscriptions.rs | 2 +- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index a4b308c..7cbb507 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -49,22 +49,7 @@ pub async fn subscribe( HttpResponse::InternalServerError().finish(); } - - let confirmation_link = "https://there-is-no-such-domain.com/subscriptions/confirm"; - if email_client - .send_email( - new_subscriber.email, - "welcome", - &format!( - "Welcome to our newsletter!
\ - Click here to confirm your subscription.", - confirmation_link - ), - &format!( - "Welcome to our newsletter!\nVisit {} to confirm your subscription.", - confirmation_link - ), - ) + if send_confirmation_email(new_subscriber, &email_client) .await .is_err() { @@ -99,3 +84,27 @@ async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result<(), sq })?; Ok(()) } + +#[tracing::instrument( + name = "Send a confirmation email to a new subscriber", + skip(email_client, form) +)] +pub async fn send_confirmation_email( + form: NewSubscriber, + email_client: &EmailClient, +) -> Result<(), reqwest::Error> { + let confirmation_link = "https://there-is-no-such-domain.com/subscriptions/confirm"; + let plain_body = format!( + "Welcome to our newsletter!\nVisit {} to confirm your subscription.", + confirmation_link + ); + let html_body = format!( + "Welcome to our newsletter!
\ + Click here to confirm your subscription.", + confirmation_link + ); + + email_client + .send_email(form.email, "welcome", &html_body, &plain_body) + .await +} diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 9a8a456..380f418 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -126,4 +126,4 @@ async fn subscribe_sends_a_confirmation_email_with_a_link() { let text_link = get_link(&body["TextBody"].as_str().unwrap()); // The two links should be identical assert_eq!(html_link, text_link); -} \ No newline at end of file +} From 157fc16e543761a0053d89e696f5b315e1504cf2 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 21:32:18 -0700 Subject: [PATCH 07/17] ch7: mark status as pending in subscriptions handler Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 6 ++---- tests/api/subscriptions.rs | 22 +++++++++++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 7cbb507..2490d59 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -64,10 +64,8 @@ pub async fn subscribe( )] async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result<(), sqlx::Error> { sqlx::query!( - r#" - INSERT INTO subscriptions (id, email, name, subscribed_at, status) - VALUES ($1, $2, $3, $4, 'confirmed') - "#, + r#"INSERT INTO subscriptions (id, email, name, subscribed_at, status) + VALUES ($1, $2, $3, $4, 'pending_confirmation')"#, Uuid::new_v4(), form.email.as_ref(), form.name.as_ref(), diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 380f418..47df919 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -21,13 +21,33 @@ async fn subscribe_returns_200_for_valid_form_data() { // Assert assert_eq!(200, resp.status().as_u16()); - let saved = sqlx::query!("SELECT email, name FROM subscriptions",) +} + +#[tokio::test] +async fn subscribe_persists_the_new_subscriber() { + // Arrange + let app = spawn_app().await; + let body = "name=le%20guin&email=ursula_le_guin%40gmail.com"; + + // Mock + Mock::given(path("/email")) + .and(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .mount(&app.email_server) + .await; + + // Act + let _ = app.post_subscriptions(body.into()).await; + + // Assert + let saved = sqlx::query!("SELECT email, name, status FROM subscriptions",) .fetch_one(&app.db_pool) .await .expect("Failed to fetch saved subscription."); assert_eq!(saved.email, "ursula_le_guin@gmail.com"); assert_eq!(saved.name, "le guin"); + assert_eq!(saved.status, "pending_confirmation"); } #[tokio::test] From f9deb098ef4d60d8768c5e3c9055b94b0ac883b7 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 21:39:30 -0700 Subject: [PATCH 08/17] ch7: add sub confirm route and test template Signed-off-by: Jin Dong --- src/routes/mod.rs | 2 ++ src/routes/subscriptions_confirm.rs | 6 ++++++ src/startup.rs | 3 ++- tests/api/main.rs | 1 + tests/api/subscriptions_confirm.rs | 12 ++++++++++++ 5 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 src/routes/subscriptions_confirm.rs create mode 100644 tests/api/subscriptions_confirm.rs diff --git a/src/routes/mod.rs b/src/routes/mod.rs index 90ffeed..d0ddba0 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -1,5 +1,7 @@ mod health_check; mod subscriptions; +mod subscriptions_confirm; pub use health_check::*; pub use subscriptions::*; +pub use subscriptions_confirm::*; diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs new file mode 100644 index 0000000..067000d --- /dev/null +++ b/src/routes/subscriptions_confirm.rs @@ -0,0 +1,6 @@ +use actix_web::{HttpResponse, Responder}; + +#[tracing::instrument(name = "Confirm a pending subscriber")] +pub async fn confirm() -> impl Responder { + HttpResponse::Ok().finish() +} diff --git a/src/startup.rs b/src/startup.rs index 96fc9a0..eadda36 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -1,6 +1,6 @@ use crate::configuration::{DatabaseSettings, Settings}; use crate::email_client::{self, EmailClient}; -use crate::routes::{health_check, subscribe}; +use crate::routes::{confirm, health_check, subscribe}; use actix_web::dev::Server; use actix_web::{web, App, HttpServer}; @@ -67,6 +67,7 @@ fn run( .wrap(TracingLogger::default()) .route("/health_check", web::get().to(health_check)) .route("/subscriptions", web::post().to(subscribe)) + .route("/subscriptions/confirm", web::get().to(confirm)) .app_data(db_pool.clone()) .app_data(email_client.clone()) }) diff --git a/tests/api/main.rs b/tests/api/main.rs index 3b9c227..177847a 100644 --- a/tests/api/main.rs +++ b/tests/api/main.rs @@ -1,3 +1,4 @@ mod health_check; mod helpers; mod subscriptions; +mod subscriptions_confirm; diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs new file mode 100644 index 0000000..4659350 --- /dev/null +++ b/tests/api/subscriptions_confirm.rs @@ -0,0 +1,12 @@ +use crate::helpers::spawn_app; + +#[tokio::test] +async fn confirmation_without_token_are_rejected_with_a_400() { + let app = spawn_app().await; + + let resp = reqwest::get(&format!("{}/subscriptions/confirm", app.address)) + .await + .expect("Failed to execute request."); + + assert_eq!(resp.status().as_u16(), 400); +} From 2b41650e31e515b11e092dbe2e345d28ec073c6d Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 22:22:07 -0700 Subject: [PATCH 09/17] ch7: [test] subscribe + confirm Signed-off-by: Jin Dong --- src/routes/subscriptions_confirm.rs | 11 +++++-- tests/api/subscriptions_confirm.rs | 46 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs index 067000d..b2c099b 100644 --- a/src/routes/subscriptions_confirm.rs +++ b/src/routes/subscriptions_confirm.rs @@ -1,6 +1,11 @@ -use actix_web::{HttpResponse, Responder}; +use actix_web::{web, HttpResponse, Responder}; -#[tracing::instrument(name = "Confirm a pending subscriber")] -pub async fn confirm() -> impl Responder { +#[derive(serde::Deserialize)] +pub struct Parameters { + pub subscription_token: String, +} + +#[tracing::instrument(name = "Confirm a pending subscriber", skip(_params))] +pub async fn confirm(_params: web::Query) -> impl Responder { HttpResponse::Ok().finish() } diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index 4659350..2ee3411 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -1,3 +1,9 @@ +use reqwest::Url; +use wiremock::{ + matchers::{method, path}, + Mock, ResponseTemplate, +}; + use crate::helpers::spawn_app; #[tokio::test] @@ -10,3 +16,43 @@ async fn confirmation_without_token_are_rejected_with_a_400() { assert_eq!(resp.status().as_u16(), 400); } + +#[tokio::test] +async fn the_link_returned_by_subscribe_returns_a_200_if_called() { + let app = spawn_app().await; + let body = "name=le%20guin&email=ursula_le_guin%40gmail.com"; + + // Mock: if email server receives POST, return 200. + // This makes sure our confirm indeed sends an email to the email server (here, mocked). + Mock::given(path("/email")) + .and(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .mount(&app.email_server) + .await; + + // Act 1: send a POST request to /subscriptions + // The response body should contain a link to confirm the subscription. + app.post_subscriptions(body.into()).await; + let email_request = &app.email_server.received_requests().await.unwrap()[0]; + let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); + let get_link = |s: &str| { + let links: Vec<_> = linkify::LinkFinder::new() + .links(s) + .filter(|l| *l.kind() == linkify::LinkKind::Url) + .collect(); + assert_eq!(links.len(), 1); + links[0].as_str().to_owned() + }; + + // Extract the confirmation link + let raw_confirmation_link = &get_link(&body["HtmlBody"].as_str().unwrap()); + let confirmation_link = Url::parse(raw_confirmation_link).unwrap(); + // Let's make sure we don't call random APIs on the web + assert_eq!(confirmation_link.host_str().unwrap(), "127.0.0.1"); + + // Act 2: call the returned link should send a GET to /subscriptions/confirm + let resp = reqwest::get(confirmation_link).await.unwrap(); + + // Assert + assert_eq!(resp.status().as_u16(), 200); +} From 06caedbadc0cf64730659db658d8267eb73edc9b Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 22:33:10 -0700 Subject: [PATCH 10/17] ch7: add base_url to config and actix context Signed-off-by: Jin Dong --- configuration/local.yaml | 1 + spec.yaml | 5 +++++ src/configuration.rs | 1 + src/routes/subscriptions.rs | 8 +++++--- src/startup.rs | 11 ++++++++++- tests/api/helpers.rs | 5 ++++- tests/api/subscriptions_confirm.rs | 4 +++- 7 files changed, 29 insertions(+), 6 deletions(-) diff --git a/configuration/local.yaml b/configuration/local.yaml index 3b77405..974d53d 100644 --- a/configuration/local.yaml +++ b/configuration/local.yaml @@ -1,4 +1,5 @@ application: host: 127.0.0.1 + base_url: "http://127.0.0.1" database: require_ssl: false \ No newline at end of file diff --git a/spec.yaml b/spec.yaml index e2de5e7..c75da7b 100644 --- a/spec.yaml +++ b/spec.yaml @@ -27,6 +27,11 @@ services: routes: - path: / envs: + # We use DO's APP_URL to inject the dynamically + # provisioned base url as an environment variable + - key: APP_APPLICATION__BASE_URL + scope: RUN_TIME + value: ${APP_URL} - key: APP_DATABASE__USERNAME scope: RUN_TIME value: ${newsletter.USERNAME} diff --git a/src/configuration.rs b/src/configuration.rs index 7bed900..affc8c6 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -28,6 +28,7 @@ pub struct DatabaseSettings { pub struct ApplicationSettings { pub host: String, pub port: u16, + pub base_url: String, } #[derive(serde::Deserialize, Clone)] diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 2490d59..bcb8a2b 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -5,7 +5,7 @@ use uuid::Uuid; use crate::{ domain::{NewSubscriber, SubscriberEmail, SubscriberName}, - email_client::EmailClient, + email_client::EmailClient, startup::ApplicationBaseUrl, }; #[derive(serde::Deserialize)] @@ -37,6 +37,7 @@ pub async fn subscribe( form: web::Form, db_pool: web::Data, email_client: web::Data, + base_url: web::Data, ) -> impl Responder { // `web::Form` is a wrapper around `FormData` // `form.0` gives us access to the underlying `FormData` @@ -49,7 +50,7 @@ pub async fn subscribe( HttpResponse::InternalServerError().finish(); } - if send_confirmation_email(new_subscriber, &email_client) + if send_confirmation_email(new_subscriber, &email_client, &base_url.0) .await .is_err() { @@ -90,8 +91,9 @@ async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result<(), sq pub async fn send_confirmation_email( form: NewSubscriber, email_client: &EmailClient, + base_url: &str, ) -> Result<(), reqwest::Error> { - let confirmation_link = "https://there-is-no-such-domain.com/subscriptions/confirm"; + let confirmation_link = format!("{}/subscriptions/confirm", base_url); let plain_body = format!( "Welcome to our newsletter!\nVisit {} to confirm your subscription.", confirmation_link diff --git a/src/startup.rs b/src/startup.rs index eadda36..54d2847 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -37,7 +37,7 @@ impl Application { let listener = TcpListener::bind(address)?; let port = listener.local_addr().unwrap().port(); - let server = run(listener, connection_pool, email_client)?; + let server = run(listener, connection_pool, email_client, configuration.application.base_url,)?; Ok(Self { port, server }) } @@ -53,11 +53,19 @@ impl Application { } } +// We need to define a wrapper type in order to retrieve the URL +// in the `subscribe` handler. +// Retrieval from the context, in actix-web, is type-based: using +// a raw `String` would expose us to conflicts. +#[derive(Debug)] +pub struct ApplicationBaseUrl(pub String); + // `async` is no longer needed as we don't have .await calls. fn run( listener: TcpListener, db_pool: PgPool, email_client: email_client::EmailClient, + base_url: String, ) -> Result { // create a ARC pointer to the DB connection let db_pool = web::Data::new(db_pool); @@ -70,6 +78,7 @@ fn run( .route("/subscriptions/confirm", web::get().to(confirm)) .app_data(db_pool.clone()) .app_data(email_client.clone()) + .app_data(web::Data::new(ApplicationBaseUrl(base_url.clone()))) }) .listen(listener)? .run(); diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index afee64a..89411e3 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -8,6 +8,7 @@ use zero2prod::telemetry::{get_subscriber, init_subscriber}; pub struct TestApp { pub address: String, + pub port: u16, pub db_pool: PgPool, pub email_server: MockServer, } @@ -63,13 +64,15 @@ pub async fn spawn_app() -> TestApp { let app = Application::build(configuration.clone()) .await .expect("Failed to build app."); - let address = format!("http://127.0.0.1:{}", app.port()); + let app_port = app.port(); + let address = format!("http://127.0.0.1:{}", app_port); // Launch the server as a background task // tokio::spawn returns a handle to the spawned future, let _ = tokio::spawn(app.run_until_stopped()); TestApp { address, + port: app_port, db_pool: get_connection_pool(&configuration.database), email_server, } diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index 2ee3411..f751408 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -46,11 +46,13 @@ async fn the_link_returned_by_subscribe_returns_a_200_if_called() { // Extract the confirmation link let raw_confirmation_link = &get_link(&body["HtmlBody"].as_str().unwrap()); - let confirmation_link = Url::parse(raw_confirmation_link).unwrap(); + let mut confirmation_link = Url::parse(raw_confirmation_link).unwrap(); // Let's make sure we don't call random APIs on the web assert_eq!(confirmation_link.host_str().unwrap(), "127.0.0.1"); // Act 2: call the returned link should send a GET to /subscriptions/confirm + // Let's rewrite the URL to include the port first + confirmation_link.set_port(Some(app.port)).unwrap(); let resp = reqwest::get(confirmation_link).await.unwrap(); // Assert From b54b733713745360239afed0432668b0b77b8393 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 22:41:01 -0700 Subject: [PATCH 11/17] ch7: subscriptions handler adds a random token in confirm link Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 8 ++++++-- src/startup.rs | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index bcb8a2b..10db7eb 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -5,7 +5,8 @@ use uuid::Uuid; use crate::{ domain::{NewSubscriber, SubscriberEmail, SubscriberName}, - email_client::EmailClient, startup::ApplicationBaseUrl, + email_client::EmailClient, + startup::ApplicationBaseUrl, }; #[derive(serde::Deserialize)] @@ -93,7 +94,10 @@ pub async fn send_confirmation_email( email_client: &EmailClient, base_url: &str, ) -> Result<(), reqwest::Error> { - let confirmation_link = format!("{}/subscriptions/confirm", base_url); + let confirmation_link = format!( + "{}/subscriptions/confirm?subscription_token=mytoken", + base_url + ); let plain_body = format!( "Welcome to our newsletter!\nVisit {} to confirm your subscription.", confirmation_link diff --git a/src/startup.rs b/src/startup.rs index 54d2847..9c65b1a 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -37,7 +37,12 @@ impl Application { let listener = TcpListener::bind(address)?; let port = listener.local_addr().unwrap().port(); - let server = run(listener, connection_pool, email_client, configuration.application.base_url,)?; + let server = run( + listener, + connection_pool, + email_client, + configuration.application.base_url, + )?; Ok(Self { port, server }) } From cf40244ddef303fda7aeae80cadd2bc850bcbddf Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sat, 30 Mar 2024 22:51:04 -0700 Subject: [PATCH 12/17] ch7: [refactor] dedup link parsing from req body Signed-off-by: Jin Dong --- tests/api/helpers.rs | 28 ++++++++++++++++++++++++++++ tests/api/subscriptions.rs | 15 ++------------- tests/api/subscriptions_confirm.rs | 23 ++--------------------- 3 files changed, 32 insertions(+), 34 deletions(-) diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index 89411e3..0d8c9b8 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -13,6 +13,11 @@ pub struct TestApp { pub email_server: MockServer, } +pub struct ConfirmationLinks { + pub html: reqwest::Url, + pub plain_text: reqwest::Url, +} + impl TestApp { pub async fn post_subscriptions(&self, body: String) -> reqwest::Response { reqwest::Client::new() @@ -23,6 +28,29 @@ impl TestApp { .await .expect("Failed to execute request.") } + + pub fn get_configuration_links(&self, email_request: &wiremock::Request) -> ConfirmationLinks { + let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); + let get_link = |s: &str| { + let links: Vec<_> = linkify::LinkFinder::new() + .links(s) + .filter(|l| *l.kind() == linkify::LinkKind::Url) + .collect(); + assert_eq!(links.len(), 1); + let raw_link = links[0].as_str().to_owned(); + let mut confirmation_link = reqwest::Url::parse(&raw_link).unwrap(); + // Let's make sure we don't call random APIs on the web + assert_eq!(confirmation_link.host_str().unwrap(), "127.0.0.1"); + confirmation_link.set_port(Some(self.port)).unwrap(); + confirmation_link + }; + + // Extract the confirmation link + let html = get_link(&body["HtmlBody"].as_str().unwrap()); + let plain_text = get_link(&body["TextBody"].as_str().unwrap()); + + ConfirmationLinks { html, plain_text } + } } // Ensure that the `tracing` stack is only initialized once using `once_cell`. diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 47df919..e3dbcff 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -132,18 +132,7 @@ async fn subscribe_sends_a_confirmation_email_with_a_link() { // Assert let email_request = &app.email_server.received_requests().await.unwrap()[0]; - let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); - let get_link = |s: &str| { - let links: Vec<_> = linkify::LinkFinder::new() - .links(s) - .filter(|l| *l.kind() == linkify::LinkKind::Url) - .collect(); - assert_eq!(links.len(), 1); - links[0].as_str().to_owned() - }; - - let html_link = get_link(&body["HtmlBody"].as_str().unwrap()); - let text_link = get_link(&body["TextBody"].as_str().unwrap()); + let link = app.get_configuration_links(email_request); // The two links should be identical - assert_eq!(html_link, text_link); + assert_eq!(link.html, link.plain_text); } diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index f751408..8e8edda 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -1,4 +1,3 @@ -use reqwest::Url; use wiremock::{ matchers::{method, path}, Mock, ResponseTemplate, @@ -34,26 +33,8 @@ async fn the_link_returned_by_subscribe_returns_a_200_if_called() { // The response body should contain a link to confirm the subscription. app.post_subscriptions(body.into()).await; let email_request = &app.email_server.received_requests().await.unwrap()[0]; - let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); - let get_link = |s: &str| { - let links: Vec<_> = linkify::LinkFinder::new() - .links(s) - .filter(|l| *l.kind() == linkify::LinkKind::Url) - .collect(); - assert_eq!(links.len(), 1); - links[0].as_str().to_owned() - }; - - // Extract the confirmation link - let raw_confirmation_link = &get_link(&body["HtmlBody"].as_str().unwrap()); - let mut confirmation_link = Url::parse(raw_confirmation_link).unwrap(); - // Let's make sure we don't call random APIs on the web - assert_eq!(confirmation_link.host_str().unwrap(), "127.0.0.1"); - - // Act 2: call the returned link should send a GET to /subscriptions/confirm - // Let's rewrite the URL to include the port first - confirmation_link.set_port(Some(app.port)).unwrap(); - let resp = reqwest::get(confirmation_link).await.unwrap(); + let link = app.get_configuration_links(email_request); + let resp = reqwest::get(link.html).await.unwrap(); // Assert assert_eq!(resp.status().as_u16(), 200); From d7352dbcde67a51c1e0872cc83a3867594e52ece Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sun, 31 Mar 2024 11:54:34 -0700 Subject: [PATCH 13/17] ch7: [test] token validation in confirm Signed-off-by: Jin Dong --- tests/api/subscriptions_confirm.rs | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index 8e8edda..aa9d66f 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -39,3 +39,34 @@ async fn the_link_returned_by_subscribe_returns_a_200_if_called() { // Assert assert_eq!(resp.status().as_u16(), 200); } + +#[tokio::test] +async fn clicking_on_the_confirmation_link_confirms_a_subscriber() { + let app = spawn_app().await; + let body = "name=le%20guin&email=ursula_le_guin%40gmail.com"; + + // Mock: if email server receives POST, return 200. + // This makes sure our confirm indeed sends an email to the email server (here, mocked). + Mock::given(path("/email")) + .and(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .mount(&app.email_server) + .await; + + // Act 1: send a POST request to /subscriptions + // The response body should contain a link to confirm the subscription. + app.post_subscriptions(body.into()).await; + let email_request = &app.email_server.received_requests().await.unwrap()[0]; + let link = app.get_configuration_links(email_request); + let _ = reqwest::get(link.html).await.unwrap().error_for_status().unwrap(); + + // Assert + let saved = sqlx::query!("SELECT email, name, status FROM subscriptions",) + .fetch_one(&app.db_pool) + .await + .expect("Failed to fetch saved subscription"); + + assert_eq!(saved.email, "ursula_le_guin@gmail.com"); + assert_eq!(saved.name, "le guin"); + assert_eq!(saved.status, "confirmed"); +} From 769270dd7e4456e37650e0396c1a85dbfa510d2f Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sun, 31 Mar 2024 11:59:32 -0700 Subject: [PATCH 14/17] ch7: move token up as a param Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 9 +++++---- tests/api/subscriptions_confirm.rs | 6 +++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 10db7eb..467ab2b 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -51,7 +51,7 @@ pub async fn subscribe( HttpResponse::InternalServerError().finish(); } - if send_confirmation_email(new_subscriber, &email_client, &base_url.0) + if send_confirmation_email(new_subscriber, &email_client, &base_url.0, "mytoken") .await .is_err() { @@ -87,16 +87,17 @@ async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result<(), sq #[tracing::instrument( name = "Send a confirmation email to a new subscriber", - skip(email_client, form) + skip(email_client, form, base_url, subscription_token) )] pub async fn send_confirmation_email( form: NewSubscriber, email_client: &EmailClient, base_url: &str, + subscription_token: &str, ) -> Result<(), reqwest::Error> { let confirmation_link = format!( - "{}/subscriptions/confirm?subscription_token=mytoken", - base_url + "{}/subscriptions/confirm?subscription_token={}", + base_url, subscription_token ); let plain_body = format!( "Welcome to our newsletter!\nVisit {} to confirm your subscription.", diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index aa9d66f..b136c0d 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -58,7 +58,11 @@ async fn clicking_on_the_confirmation_link_confirms_a_subscriber() { app.post_subscriptions(body.into()).await; let email_request = &app.email_server.received_requests().await.unwrap()[0]; let link = app.get_configuration_links(email_request); - let _ = reqwest::get(link.html).await.unwrap().error_for_status().unwrap(); + let _ = reqwest::get(link.html) + .await + .unwrap() + .error_for_status() + .unwrap(); // Assert let saved = sqlx::query!("SELECT email, name, status FROM subscriptions",) From d8fe1e489a6d5af4a4e9029a833fda7251bd0299 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sun, 31 Mar 2024 14:02:25 -0700 Subject: [PATCH 15/17] ch7: insert subscription token in POST /subscriptions Signed-off-by: Jin Dong --- Cargo.lock | 1 + Cargo.toml | 1 + src/routes/subscriptions.rs | 46 +++++++++++++++++++++++++++++++++---- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 483def1..62d3ba3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3465,6 +3465,7 @@ dependencies = [ "once_cell", "quickcheck", "quickcheck_macros", + "rand 0.8.5", "reqwest", "secrecy", "serde", diff --git a/Cargo.toml b/Cargo.toml index 4141cc4..51bd32b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ actix-web = "4.5.1" chrono = { version = "0.4.35", default-features = false, features = ["clock"] } config = "0.14.0" log = "0.4.21" +rand = { version = "0.8.5", features = ["std_rng"] } reqwest = { version = "0.12.1", default-features = false, features = ["json", "rustls-tls"] } secrecy = { version = "0.8.0", features = ["serde"] } serde = { version = "1.0.197", features = ["derive"] } diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 467ab2b..4d0dce5 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -1,5 +1,6 @@ use actix_web::{web, HttpResponse, Responder}; use chrono::Utc; +use rand::{distributions::Alphanumeric, thread_rng, Rng}; use sqlx::PgPool; use uuid::Uuid; @@ -47,8 +48,14 @@ pub async fn subscribe( Err(_) => return HttpResponse::BadRequest().finish(), }; - if insert_subscriber(&new_subscriber, &db_pool).await.is_err() { - HttpResponse::InternalServerError().finish(); + let subscriber_id = match insert_subscriber(&new_subscriber, &db_pool).await { + Ok(id) => id, + Err(_) => return HttpResponse::InternalServerError().finish(), + }; + let subscription_token = generate_subscription_token(); + + if store_token(&db_pool, subscriber_id, &subscription_token).await.is_err() { + return HttpResponse::InternalServerError().finish(); } if send_confirmation_email(new_subscriber, &email_client, &base_url.0, "mytoken") @@ -64,11 +71,12 @@ pub async fn subscribe( name = "Saving new subscriber details in the database", skip(form, pool) )] -async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result<(), sqlx::Error> { +async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result { + let uuid = Uuid::new_v4(); sqlx::query!( r#"INSERT INTO subscriptions (id, email, name, subscribed_at, status) VALUES ($1, $2, $3, $4, 'pending_confirmation')"#, - Uuid::new_v4(), + uuid, form.email.as_ref(), form.name.as_ref(), Utc::now() @@ -82,7 +90,7 @@ async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result<(), sq // if the function failed, returning a sqlx::Error // We will talk about error handling in depth later! })?; - Ok(()) + Ok(uuid) } #[tracing::instrument( @@ -113,3 +121,31 @@ pub async fn send_confirmation_email( .send_email(form.email, "welcome", &html_body, &plain_body) .await } + +#[tracing::instrument( + name = "Storing subscription token in the database", + skip(pool, subscriber_token) +)] +pub async fn store_token(pool: &PgPool, subscriber_id: Uuid, subscriber_token: &str) -> Result<(), sqlx::Error>{ + sqlx::query!( + r#"INSERT INTO subscription_tokens (subscription_token, subscriber_id) + VALUES ($1, $2)"#, + subscriber_token, + subscriber_id + ) + .execute(pool) + .await + .map_err(|e| { + tracing::error!("Failed to execute query: {:?}", e); + e + })?; + Ok(()) +} + +fn generate_subscription_token() -> String { + let mut rng = thread_rng(); + std::iter::repeat_with(|| rng.sample(Alphanumeric)) + .map(char::from) + .take(25) + .collect() +} From 1044b8c0ae9634489c96f9939c5496e72c23d51a Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sun, 31 Mar 2024 14:28:32 -0700 Subject: [PATCH 16/17] ch7: /subscriptions/confirm check token and update status Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 22 +++++++--- src/routes/subscriptions_confirm.rs | 67 +++++++++++++++++++++++++++-- 2 files changed, 80 insertions(+), 9 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 4d0dce5..3e36def 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -54,13 +54,21 @@ pub async fn subscribe( }; let subscription_token = generate_subscription_token(); - if store_token(&db_pool, subscriber_id, &subscription_token).await.is_err() { + if store_token(&db_pool, subscriber_id, &subscription_token) + .await + .is_err() + { return HttpResponse::InternalServerError().finish(); } - if send_confirmation_email(new_subscriber, &email_client, &base_url.0, "mytoken") - .await - .is_err() + if send_confirmation_email( + new_subscriber, + &email_client, + &base_url.0, + &subscription_token, + ) + .await + .is_err() { return HttpResponse::InternalServerError().finish(); } @@ -126,7 +134,11 @@ pub async fn send_confirmation_email( name = "Storing subscription token in the database", skip(pool, subscriber_token) )] -pub async fn store_token(pool: &PgPool, subscriber_id: Uuid, subscriber_token: &str) -> Result<(), sqlx::Error>{ +pub async fn store_token( + pool: &PgPool, + subscriber_id: Uuid, + subscriber_token: &str, +) -> Result<(), sqlx::Error> { sqlx::query!( r#"INSERT INTO subscription_tokens (subscription_token, subscriber_id) VALUES ($1, $2)"#, diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs index b2c099b..fde04bc 100644 --- a/src/routes/subscriptions_confirm.rs +++ b/src/routes/subscriptions_confirm.rs @@ -1,11 +1,70 @@ use actix_web::{web, HttpResponse, Responder}; +use sqlx::PgPool; +use uuid::Uuid; -#[derive(serde::Deserialize)] +#[derive(serde::Deserialize, Debug)] pub struct Parameters { pub subscription_token: String, } -#[tracing::instrument(name = "Confirm a pending subscriber", skip(_params))] -pub async fn confirm(_params: web::Query) -> impl Responder { - HttpResponse::Ok().finish() +#[tracing::instrument(name = "Confirm a pending subscriber", skip(params, db_pool))] +pub async fn confirm(params: web::Query, db_pool: web::Data) -> impl Responder { + let id = match get_subscriber_id_from_token(&db_pool, ¶ms.subscription_token).await { + Ok(id) => id, + Err(_) => return HttpResponse::BadRequest().finish(), + }; + + match id { + None => HttpResponse::Unauthorized().finish(), + Some(subscriber_id) => { + if confirm_subscriber(&db_pool, subscriber_id).await.is_err() { + return HttpResponse::InternalServerError().finish(); + } + HttpResponse::Ok().finish() + } + } +} + +#[tracing::instrument(name = "Mark subcriber as confirmed", skip(id, pool))] +pub async fn confirm_subscriber(pool: &PgPool, id: Uuid) -> Result<(), sqlx::Error> { + sqlx::query!( + r#" + UPDATE subscriptions + SET status = 'confirmed' + WHERE id = $1 + "#, + id + ) + .execute(pool) + .await + .map_err(|e| { + tracing::error!( + "Failed to update subscriber status in the database: {:?}", + e + ); + e + })?; + Ok(()) +} + +#[tracing::instrument(name = "Retrieving subscriber ID from the database", skip(token, pool))] +async fn get_subscriber_id_from_token( + pool: &PgPool, + token: &str, +) -> Result, sqlx::Error> { + let result = sqlx::query!( + r#" + SELECT subscriber_id + FROM subscription_tokens + WHERE subscription_token = $1 + "#, + token, + ) + .fetch_optional(pool) + .await + .map_err(|e| { + tracing::error!("Failed to execute query: {:?}", e); + e + })?; + Ok(result.map(|r| r.subscriber_id)) } From e5bf8fc47bcd197c1098b9b2360b0785fcc6c5a0 Mon Sep 17 00:00:00 2001 From: Jin Dong Date: Sun, 31 Mar 2024 14:42:36 -0700 Subject: [PATCH 17/17] ch7: change POST /subscriptions to sqlx Transaction Signed-off-by: Jin Dong --- src/routes/subscriptions.rs | 43 ++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 3e36def..256c611 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -1,7 +1,7 @@ use actix_web::{web, HttpResponse, Responder}; use chrono::Utc; use rand::{distributions::Alphanumeric, thread_rng, Rng}; -use sqlx::PgPool; +use sqlx::{Executor, PgPool, Postgres, Transaction}; use uuid::Uuid; use crate::{ @@ -48,13 +48,18 @@ pub async fn subscribe( Err(_) => return HttpResponse::BadRequest().finish(), }; - let subscriber_id = match insert_subscriber(&new_subscriber, &db_pool).await { + let mut txn = match db_pool.begin().await { + Ok(t) => t, + Err(_) => return HttpResponse::InternalServerError().finish(), + }; + + let subscriber_id = match insert_subscriber(&new_subscriber, &mut txn).await { Ok(id) => id, Err(_) => return HttpResponse::InternalServerError().finish(), }; let subscription_token = generate_subscription_token(); - if store_token(&db_pool, subscriber_id, &subscription_token) + if store_token(&mut txn, subscriber_id, &subscription_token) .await .is_err() { @@ -72,26 +77,32 @@ pub async fn subscribe( { return HttpResponse::InternalServerError().finish(); } + + if txn.commit().await.is_err() { + return HttpResponse::InternalServerError().finish(); + } + HttpResponse::Ok().finish() } #[tracing::instrument( name = "Saving new subscriber details in the database", - skip(form, pool) + skip(form, txn) )] -async fn insert_subscriber(form: &NewSubscriber, pool: &PgPool) -> Result { +async fn insert_subscriber( + form: &NewSubscriber, + txn: &mut Transaction<'_, Postgres>, +) -> Result { let uuid = Uuid::new_v4(); - sqlx::query!( + let query = sqlx::query!( r#"INSERT INTO subscriptions (id, email, name, subscribed_at, status) VALUES ($1, $2, $3, $4, 'pending_confirmation')"#, uuid, form.email.as_ref(), form.name.as_ref(), Utc::now() - ) - .execute(pool) - .await - .map_err(|e| { + ); + txn.execute(query).await.map_err(|e| { tracing::error!("Failed to execute query: {:?}", e); e // Using the `?` operator to return early @@ -132,22 +143,20 @@ pub async fn send_confirmation_email( #[tracing::instrument( name = "Storing subscription token in the database", - skip(pool, subscriber_token) + skip(txn, subscriber_token) )] pub async fn store_token( - pool: &PgPool, + txn: &mut Transaction<'_, Postgres>, subscriber_id: Uuid, subscriber_token: &str, ) -> Result<(), sqlx::Error> { - sqlx::query!( + let query = sqlx::query!( r#"INSERT INTO subscription_tokens (subscription_token, subscriber_id) VALUES ($1, $2)"#, subscriber_token, subscriber_id - ) - .execute(pool) - .await - .map_err(|e| { + ); + txn.execute(query).await.map_err(|e| { tracing::error!("Failed to execute query: {:?}", e); e })?;