From 6dfc9a0f3e560df57c1ad50a304841bdf982bc7b Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Mon, 4 Mar 2024 21:25:55 +0100 Subject: [PATCH 01/10] feat: add subscription_tokens table --- .../20240304202111_create_subscription_tokens_table.sql | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 migrations/20240304202111_create_subscription_tokens_table.sql diff --git a/migrations/20240304202111_create_subscription_tokens_table.sql b/migrations/20240304202111_create_subscription_tokens_table.sql new file mode 100644 index 0000000..60e260e --- /dev/null +++ b/migrations/20240304202111_create_subscription_tokens_table.sql @@ -0,0 +1,6 @@ +-- Create Subscription Tokens Table +CREATE TABLE subscription_tokens( +subscription_token TEXT NOT NULL, +subscriber_id uuid NOT NULL REFERENCES subscriptions (id), +PRIMARY KEY (subscription_token) +); From 0427df8656044bac8d3cc6ab19d5d8cb1a2b03a5 Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Mon, 4 Mar 2024 21:49:08 +0100 Subject: [PATCH 02/10] feat: actually send email --- src/routes/subscriptions.rs | 23 ++++++++++++++++------- tests/api/helpers.rs | 11 +++++++---- tests/api/subscriptions.rs | 29 ++++++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 12 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index e59b786..6dd1fbb 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -30,6 +30,7 @@ impl TryFrom for NewSubscriber { } } +// TODO: remove request_id? #[tracing::instrument( name = "Adding a new subscriber", skip(form, db_pool, email_client), @@ -52,14 +53,22 @@ pub async fn subscribe( return (StatusCode::BAD_REQUEST, "Invalid subscription.").into_response(); } }; - match insert_subscriber(&db_pool, &new_subscriber).await { - Ok(_) => { - return (StatusCode::OK,).into_response(); - } - Err(_) => { - return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").into_response(); - } + if insert_subscriber(&db_pool, &new_subscriber).await.is_err() { + return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").into_response(); } + if email_client + .send_email( + new_subscriber.email, + "Welcome!", + "Welcome to our newsletter!", + "Welcome to our newsletter!", + ) + .await + .is_err() + { + return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").into_response(); + } + return (StatusCode::OK,).into_response(); } #[tracing::instrument( diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index ccee1ce..9010e59 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -4,6 +4,7 @@ use learn_axum::telemetry::{get_subscriber, init_subscriber}; use once_cell::sync::Lazy; use sqlx::{Connection, Executor, PgConnection, PgPool}; use uuid::Uuid; +use wiremock::MockServer; /// Ensure that the `tracing` stack is only initialised once using `once_cell` static TRACING: Lazy<()> = Lazy::new(|| { @@ -25,6 +26,7 @@ static TRACING: Lazy<()> = Lazy::new(|| { pub struct TestApp { pub address: String, pub db_pool: PgPool, + pub email_server: MockServer, } impl TestApp { @@ -51,9 +53,8 @@ pub async fn spawn_app() -> TestApp { // All other invocations will instead skip execution. Lazy::force(&TRACING); - // TODO: - // // Launch a mock server to stand in for Postmark's API - // let email_server = MockServer::start().await; + // Launch a mock server to stand in for Postmark's API + let email_server = MockServer::start().await; // Randomise configuration to ensure test isolation let configuration = { @@ -62,6 +63,8 @@ pub async fn spawn_app() -> TestApp { c.database.name = Uuid::new_v4().to_string(); // Use a random OS port c.application.port = 0; + // Use the mock server as email API + c.email_client.base_url = email_server.uri(); c }; @@ -81,7 +84,7 @@ pub async fn spawn_app() -> TestApp { address, // port: application_port, db_pool: connection_pool, - // email_server, + email_server, // test_user: TestUser::generate(), // api_client: client, // email_client: configuration.email_client.client(), diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 276fd09..bfa8587 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -1,4 +1,26 @@ -use crate::helpers::{spawn_app, TestApp}; +use crate::helpers::spawn_app; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, ResponseTemplate}; + +#[tokio::test] +async fn subscribe_sends_a_confirmation_email_for_valid_data() { + // 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)) + .expect(1) + .mount(&app.email_server) + .await; + + // Act + app.post_subscriptions(body.into()).await; + + // Assert + // Mock asserts on drop +} #[tokio::test] async fn subscribe_returns_a_422_when_data_is_missing() { @@ -29,6 +51,11 @@ async fn subscribe_returns_a_200_for_valid_form_data() { // 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)) + .mount(&app.email_server) + .await; // Act let response = app.post_subscriptions(body.into()).await; From 90fc6abf19245c2063b5431e0e28fc6f31f9471f Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Tue, 5 Mar 2024 10:12:34 +0100 Subject: [PATCH 03/10] feat: add confirmation link to email --- Cargo.lock | 10 ++++++++++ Cargo.toml | 6 +----- src/routes/subscriptions.rs | 12 ++++++++++-- tests/api/subscriptions.rs | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 14bb877..6ae4c9e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1112,6 +1112,7 @@ dependencies = [ "config", "fake", "hyper 1.2.0", + "linkify", "once_cell", "quickcheck", "quickcheck_macros", @@ -1165,6 +1166,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.3.8" diff --git a/Cargo.toml b/Cargo.toml index 5c621aa..fe81084 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,12 +19,10 @@ hyper = { version = "1.2.0", features = ["full"] } serde = { version = "1.0", features = ["derive"] } serde_json = "1" serde-aux = "4" -# serde_with = "3" # Axum axum = { version = "0.7" } tower = { version = "0.4" } tower-http = { version = "0.5", features = ["trace", "request-id", "util"] } -# tower-cookies = "0.10" # Others config = "0.14" uuid = { version = "1", features = ["v4", "fast-rng"] } @@ -38,9 +36,6 @@ unicode-segmentation = "1" strum_macros = "0.26" validator = "0.16" -# async-trait = "0.1" -# strum_macros = "0.25" - [dependencies.sqlx] version = "0.7" default-features = false @@ -68,3 +63,4 @@ quickcheck_macros = "1.0.0" rand = "0.8.5" wiremock = "0.6.0" serde_json = "1" +linkify = "0.10" diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 6dd1fbb..7167657 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -56,12 +56,20 @@ pub async fn subscribe( if insert_subscriber(&db_pool, &new_subscriber).await.is_err() { return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").into_response(); } + let confirmation_link = "https://my-api.com/subscriptions/confirm"; if email_client .send_email( new_subscriber.email, "Welcome!", - "Welcome to our newsletter!", - "Welcome to our newsletter!", + &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() diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index bfa8587..e9bae61 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -2,6 +2,41 @@ use crate::helpers::spawn_app; use wiremock::matchers::{method, path}; use wiremock::{Mock, ResponseTemplate}; +#[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)) + .mount(&app.email_server) + .await; + + // Act + app.post_subscriptions(body.into()).await; + + // Assert + // Get the first intercepted request + let email_request = &app.email_server.received_requests().await.unwrap()[0]; + // Parse the body as JSON, starting from raw bytes + let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); + // Extract the link from one of the request fields. + 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); +} + #[tokio::test] async fn subscribe_sends_a_confirmation_email_for_valid_data() { // Arrange From 9a310807078cfaf37bcb2c9e79944f4016abf5dd Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Tue, 5 Mar 2024 10:45:09 +0100 Subject: [PATCH 04/10] refactor: clean up subscription --- src/routes/subscriptions.rs | 40 +++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 7167657..25b3f99 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -1,5 +1,6 @@ use crate::domain::SubscriberEmail; use crate::domain::{NewSubscriber, SubscriberName}; +use crate::email_client::EmailClient; use crate::startup::AppState; use axum::extract::State; use axum::routing::post; @@ -30,6 +31,29 @@ impl TryFrom for NewSubscriber { } } +#[tracing::instrument( + name = "Send a confirmation email to a new subscriber", + skip(email_client, new_subscriber) +)] +pub async fn send_confirmation_email( + email_client: &EmailClient, + new_subscriber: NewSubscriber, +) -> Result<(), reqwest::Error> { + let confirmation_link = "https://my-api.com/subscriptions/confirm"; + let plain_body = format!( + "Welcome to our newsletter! Visit {} 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(new_subscriber.email, "Welcome!", &html_body, &plain_body) + .await +} + // TODO: remove request_id? #[tracing::instrument( name = "Adding a new subscriber", @@ -56,21 +80,7 @@ pub async fn subscribe( if insert_subscriber(&db_pool, &new_subscriber).await.is_err() { return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").into_response(); } - let confirmation_link = "https://my-api.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(&email_client, new_subscriber) .await .is_err() { From 1bc7ae4c7a87f66357d220e67764a93a1b06c600 Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Tue, 5 Mar 2024 14:06:55 +0100 Subject: [PATCH 05/10] chore: update validator to 0.17 --- Cargo.lock | 20 +++++++++++++++----- Cargo.toml | 2 +- src/domain/subscriber_email.rs | 13 ++++++++++--- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ae4c9e..2cf14cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1012,6 +1012,16 @@ dependencies = [ "unicode-normalization", ] +[[package]] +name = "idna" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "634d9b1461af396cad843f47fdba5597a4f9e6ddd4bfb6ff5d85028c25cb12f6" +dependencies = [ + "unicode-bidi", + "unicode-normalization", +] + [[package]] name = "indexmap" version = "1.9.3" @@ -2719,7 +2729,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "50bff7831e19200a85b17131d085c25d7811bc4e186efdaf54bbd132994a88cb" dependencies = [ "form_urlencoded", - "idna", + "idna 0.4.0", "percent-encoding", ] @@ -2741,12 +2751,12 @@ dependencies = [ [[package]] name = "validator" -version = "0.16.1" +version = "0.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b92f40481c04ff1f4f61f304d61793c7b56ff76ac1469f1beb199b1445b253bd" +checksum = "da339118f018cc70ebf01fafc103360528aad53717e4bf311db929cb01cb9345" dependencies = [ - "idna", - "lazy_static", + "idna 0.5.0", + "once_cell", "regex", "serde", "serde_derive", diff --git a/Cargo.toml b/Cargo.toml index fe81084..004b0f3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,7 +34,7 @@ tracing-log = "0.2" secrecy = { version = "0.8", features = ["serde"] } unicode-segmentation = "1" strum_macros = "0.26" -validator = "0.16" +validator = "0.17" [dependencies.sqlx] version = "0.7" diff --git a/src/domain/subscriber_email.rs b/src/domain/subscriber_email.rs index f2ea9c2..d6d249d 100644 --- a/src/domain/subscriber_email.rs +++ b/src/domain/subscriber_email.rs @@ -1,18 +1,25 @@ -use validator::validate_email; +use validator::ValidateEmail; #[derive(Debug, Clone)] pub struct SubscriberEmail(String); impl SubscriberEmail { pub fn parse(s: String) -> Result { - if validate_email(&s) { - Ok(Self(s)) + let result = Self(s.clone()); + if result.validate_email() { + Ok(result) } else { Err(format!("{} is not a valid subscriber email.", s)) } } } +impl ValidateEmail for SubscriberEmail { + fn as_email_string(&self) -> Option> { + Some(self.0.as_str().into()) + } +} + impl AsRef for SubscriberEmail { fn as_ref(&self) -> &str { &self.0 From 7eebcb12a256f966ebc355003c01e3634cf0fd8e Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Sat, 9 Mar 2024 14:06:47 +0100 Subject: [PATCH 06/10] feat: add confirmation --- src/routes/mod.rs | 2 ++ src/routes/subscriptions.rs | 4 +--- src/routes/subscriptions_confirm.rs | 24 ++++++++++++++++++++++++ src/startup.rs | 4 +++- tests/api/main.rs | 1 + tests/api/subscriptions.rs | 21 +++++++++++++++++++-- tests/api/subscriptions_confirm.rs | 15 +++++++++++++++ 7 files changed, 65 insertions(+), 6 deletions(-) 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.rs b/src/routes/subscriptions.rs index 25b3f99..05950be 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -100,15 +100,13 @@ pub async fn insert_subscriber( let _ = sqlx::query!( r#" INSERT INTO subscriptions (id, email, name, subscribed_at, status) - VALUES ($1, $2, $3, $4, 'confirmed') + VALUES ($1, $2, $3, $4, 'pending_confirmation') "#, Uuid::new_v4(), new_subscriber.email.as_ref(), new_subscriber.name.as_ref(), Utc::now() ) - // We use `get_ref` to get an immutable reference to the `PgConnection` - // wrapped by `web::Data`. .execute(db_pool) .await .map_err(|e| { diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs new file mode 100644 index 0000000..7958e06 --- /dev/null +++ b/src/routes/subscriptions_confirm.rs @@ -0,0 +1,24 @@ +use crate::startup::AppState; +use axum::routing::post; +use axum::{ + extract::Query, + http::StatusCode, + response::{IntoResponse, Response}, + Router, +}; + +#[derive(serde::Deserialize)] +pub struct Parameters { + subscription_token: String, +} + +#[tracing::instrument(name = "Confirm a pending subscriber", skip(_parameters))] +pub async fn confirm(Query(_parameters): Query) -> Response { + return (StatusCode::OK,).into_response(); +} + +pub fn routes_subscriptions_confirm(state: AppState) -> Router { + Router::new() + .route("/subscriptions/confirm", post(confirm)) + .with_state(state) +} diff --git a/src/startup.rs b/src/startup.rs index c491757..cb020f8 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -65,7 +65,9 @@ impl Application { }; let app = Router::new() .merge(crate::routes::routes_health_check()) - .merge(crate::routes::routes_subscriptions(state)) + // TODO: check whether state cloning is what we want + .merge(crate::routes::routes_subscriptions(state.clone())) + .merge(crate::routes::routes_subscriptions_confirm(state)) .layer( // from https://docs.rs/tower-http/0.2.5/tower_http/request_id/index.html#using-trace ServiceBuilder::new() 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.rs b/tests/api/subscriptions.rs index e9bae61..7643d7c 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -97,14 +97,31 @@ async fn subscribe_returns_a_200_for_valid_form_data() { // Assert assert_eq!(200, response.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::given(path("/email")) + .and(method("POST")) + .respond_with(ResponseTemplate::new(200)) + .mount(&app.email_server) + .await; + + // Act + 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] @@ -125,7 +142,7 @@ async fn subscribe_returns_a_400_when_fields_are_present_but_invalid() { assert_eq!( 400, response.status().as_u16(), - "The API did not return a 200 OK when the payload was {}.", + "The API did not fail with 400 when the payload was {}.", description ); } diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs new file mode 100644 index 0000000..f8011fd --- /dev/null +++ b/tests/api/subscriptions_confirm.rs @@ -0,0 +1,15 @@ +use crate::helpers::spawn_app; + +#[tokio::test] +async fn confirmations_without_token_are_rejected_with_a_405() { + // Arrange + let app = spawn_app().await; + + // Act + let response = reqwest::get(&format!("{}/subscriptions/confirm", app.address)) + .await + .unwrap(); + + // Assert + assert_eq!(response.status().as_u16(), 405); +} From 8045eb979e9a912e65a9fdacb4a356eb099397fc Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Thu, 18 Apr 2024 14:31:06 +0200 Subject: [PATCH 07/10] feat: red test implementation of email confirm --- README.md | 2 ++ configuration/local.yaml | 1 + src/configuration.rs | 1 + src/routes/subscriptions.rs | 10 +++--- src/routes/subscriptions_confirm.rs | 3 +- src/startup.rs | 12 +++++++ tests/api/helpers.rs | 6 ++-- tests/api/subscriptions_confirm.rs | 49 +++++++++++++++++++++++++++-- 8 files changed, 75 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index e1b3b2c..414656e 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,8 @@ TODO: Add general information about this project TODO: Explain usage of docker vs podman +TODO: add https://crates.io/crates/cargo-semver-checks + ## TODO: explain DB migration To migrate a already deployed and running database, use diff --git a/configuration/local.yaml b/configuration/local.yaml index 8fd67fa..d7f541e 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 diff --git a/src/configuration.rs b/src/configuration.rs index 6c8fab1..4f197dc 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -35,6 +35,7 @@ pub struct ApplicationSettings { #[serde(deserialize_with = "deserialize_number_from_string")] pub port: u16, pub host: String, + pub base_url: String, } #[derive(serde::Deserialize, Clone)] diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 05950be..fabd244 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -33,13 +33,14 @@ impl TryFrom for NewSubscriber { #[tracing::instrument( name = "Send a confirmation email to a new subscriber", - skip(email_client, new_subscriber) + skip(email_client, new_subscriber, base_url) )] pub async fn send_confirmation_email( email_client: &EmailClient, new_subscriber: NewSubscriber, + base_url: &str, ) -> Result<(), reqwest::Error> { - let confirmation_link = "https://my-api.com/subscriptions/confirm"; + let confirmation_link = format!("{}/subscriptions/confirm?subscription_token=mytoken", base_url); let plain_body = format!( "Welcome to our newsletter! Visit {} to confirm your subscription.", confirmation_link @@ -57,7 +58,7 @@ pub async fn send_confirmation_email( // TODO: remove request_id? #[tracing::instrument( name = "Adding a new subscriber", - skip(form, db_pool, email_client), + skip(form, db_pool, email_client, base_url), fields( request_id = %Uuid::new_v4(), subscriber_email = %form.email, @@ -68,6 +69,7 @@ pub async fn subscribe( State(AppState { db_pool, email_client, + base_url, }): State, Form(form): Form, ) -> Response { @@ -80,7 +82,7 @@ pub async fn subscribe( if insert_subscriber(&db_pool, &new_subscriber).await.is_err() { return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").into_response(); } - if send_confirmation_email(&email_client, new_subscriber) + if send_confirmation_email(&email_client, new_subscriber, &base_url.0) .await .is_err() { diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs index 7958e06..bc0bdd8 100644 --- a/src/routes/subscriptions_confirm.rs +++ b/src/routes/subscriptions_confirm.rs @@ -13,7 +13,8 @@ pub struct Parameters { } #[tracing::instrument(name = "Confirm a pending subscriber", skip(_parameters))] -pub async fn confirm(Query(_parameters): Query) -> Response { +pub async fn confirm(Query(_parameters): Query) -> impl IntoResponse { + println!("subscription_token: {}", _parameters.subscription_token); return (StatusCode::OK,).into_response(); } diff --git a/src/startup.rs b/src/startup.rs index cb020f8..f994fc0 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -21,10 +21,14 @@ pub struct Application { listener: TcpListener, } +#[derive(Clone)] +pub struct ApplicationBaseUrl(pub String); + #[derive(Clone)] pub struct AppState { pub db_pool: PgPool, pub email_client: EmailClient, + pub base_url: ApplicationBaseUrl, } // from https://docs.rs/tower-http/0.2.5/tower_http/request_id/index.html#using-uuids @@ -59,15 +63,23 @@ impl Application { ); let listener = TcpListener::bind(&address).await?; + // FIXME: don't clone if not necessary let state = AppState { db_pool: connection_pool.clone(), email_client: email_client.clone(), + base_url: ApplicationBaseUrl (configuration.application.base_url), }; + + // NOTE: [her] There might be a problem with the state handling, the given version + // seems to me as if it has no "outer state" - but I might obviously be wrong. + // + // Check this: https://docs.rs/axum/latest/axum/routing/struct.Router.html#merging-routers-with-state let app = Router::new() .merge(crate::routes::routes_health_check()) // TODO: check whether state cloning is what we want .merge(crate::routes::routes_subscriptions(state.clone())) .merge(crate::routes::routes_subscriptions_confirm(state)) + // .with_state(state) .layer( // from https://docs.rs/tower-http/0.2.5/tower_http/request_id/index.html#using-trace ServiceBuilder::new() diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index 9010e59..a7c598e 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -25,6 +25,7 @@ static TRACING: Lazy<()> = Lazy::new(|| { pub struct TestApp { pub address: String, + pub port: u16, pub db_pool: PgPool, pub email_server: MockServer, } @@ -75,14 +76,15 @@ pub async fn spawn_app() -> TestApp { .await .expect("Failed to build application."); // Get the port before spawning the application - let address = format!("http://127.0.0.1:{}", application.port()); + let application_port = application.port(); + let address = format!("http://127.0.0.1:{}", application_port); // Launch the application as a background task tokio::spawn(async move { application.run().await.expect("Failed to run the server") }); TestApp { address, - // port: application_port, + port: application_port, db_pool: connection_pool, email_server, // test_user: TestUser::generate(), diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index f8011fd..518c002 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -1,7 +1,51 @@ use crate::helpers::spawn_app; +use reqwest::Url; +use wiremock::{ResponseTemplate, Mock}; +use wiremock::matchers::{path, method}; #[tokio::test] -async fn confirmations_without_token_are_rejected_with_a_405() { +async fn the_link_returned_by_subscribe_returns_a_200_if_called() { + // 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)) + .mount(&app.email_server) + .await; + + 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(); + // Extract the link from one of the request fields. + 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 raw_confirmation_link = &get_link(body["HtmlBody"].as_str().unwrap()); + let mut confirmation_link = Url::parse(raw_confirmation_link).unwrap(); + assert_eq!(confirmation_link.host_str().unwrap(), "127.0.0.1"); + confirmation_link.set_port(Some(app.port)).unwrap(); + + println!("\n################################\n{}\n##########################################", confirmation_link); + + // Act + let response = reqwest::get(confirmation_link) + .await + .unwrap(); + + // Assert + assert_eq!(response.status().as_u16(), 200); +} + +#[tokio::test] +async fn confirmations_without_token_are_rejected_with_a_400() { // Arrange let app = spawn_app().await; @@ -11,5 +55,6 @@ async fn confirmations_without_token_are_rejected_with_a_405() { .unwrap(); // Assert - assert_eq!(response.status().as_u16(), 405); + assert_eq!(response.status().as_u16(), 400); } + From e1c27ca3081925ab27dd38d8216636139ef5072c Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Thu, 18 Apr 2024 23:00:50 +0200 Subject: [PATCH 08/10] fix: set route's method to GET --- src/routes/subscriptions.rs | 11 ++++++----- src/routes/subscriptions_confirm.rs | 14 ++++++-------- src/startup.rs | 14 +++++--------- tests/api/subscriptions_confirm.rs | 17 ++++++----------- 4 files changed, 23 insertions(+), 33 deletions(-) diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index fabd244..e276b1b 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -40,7 +40,10 @@ pub async fn send_confirmation_email( new_subscriber: NewSubscriber, base_url: &str, ) -> Result<(), reqwest::Error> { - let confirmation_link = format!("{}/subscriptions/confirm?subscription_token=mytoken", base_url); + let confirmation_link = format!( + "{}/subscriptions/confirm?subscription_token=mytoken", + base_url + ); let plain_body = format!( "Welcome to our newsletter! Visit {} to confirm your subscription.", confirmation_link @@ -118,8 +121,6 @@ pub async fn insert_subscriber( Ok(()) } -pub fn routes_subscriptions(state: AppState) -> Router { - Router::new() - .route("/subscriptions", post(subscribe)) - .with_state(state) +pub fn routes_subscriptions() -> Router { + Router::new().route("/subscriptions", post(subscribe)) } diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs index bc0bdd8..ca5ed06 100644 --- a/src/routes/subscriptions_confirm.rs +++ b/src/routes/subscriptions_confirm.rs @@ -1,5 +1,5 @@ use crate::startup::AppState; -use axum::routing::post; +use axum::routing::get; use axum::{ extract::Query, http::StatusCode, @@ -12,14 +12,12 @@ pub struct Parameters { subscription_token: String, } -#[tracing::instrument(name = "Confirm a pending subscriber", skip(_parameters))] -pub async fn confirm(Query(_parameters): Query) -> impl IntoResponse { +// #[tracing::instrument(name = "Confirm a pending subscriber", skip(_parameters))] +pub async fn confirm(Query(_parameters): Query) -> Response { println!("subscription_token: {}", _parameters.subscription_token); - return (StatusCode::OK,).into_response(); + (StatusCode::OK,).into_response() } -pub fn routes_subscriptions_confirm(state: AppState) -> Router { - Router::new() - .route("/subscriptions/confirm", post(confirm)) - .with_state(state) +pub fn routes_subscriptions_confirm() -> Router { + Router::new().route("/subscriptions/confirm", get(confirm)) } diff --git a/src/startup.rs b/src/startup.rs index f994fc0..93397c0 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -67,19 +67,15 @@ impl Application { let state = AppState { db_pool: connection_pool.clone(), email_client: email_client.clone(), - base_url: ApplicationBaseUrl (configuration.application.base_url), + base_url: ApplicationBaseUrl(configuration.application.base_url), }; - // NOTE: [her] There might be a problem with the state handling, the given version - // seems to me as if it has no "outer state" - but I might obviously be wrong. - // - // Check this: https://docs.rs/axum/latest/axum/routing/struct.Router.html#merging-routers-with-state let app = Router::new() - .merge(crate::routes::routes_health_check()) // TODO: check whether state cloning is what we want - .merge(crate::routes::routes_subscriptions(state.clone())) - .merge(crate::routes::routes_subscriptions_confirm(state)) - // .with_state(state) + .merge(crate::routes::routes_subscriptions()) + .merge(crate::routes::routes_subscriptions_confirm()) + .with_state(state) + .merge(crate::routes::routes_health_check()) .layer( // from https://docs.rs/tower-http/0.2.5/tower_http/request_id/index.html#using-trace ServiceBuilder::new() diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index 518c002..3ed4cfd 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -1,7 +1,7 @@ use crate::helpers::spawn_app; use reqwest::Url; -use wiremock::{ResponseTemplate, Mock}; -use wiremock::matchers::{path, method}; +use wiremock::matchers::{method, path}; +use wiremock::{Mock, ResponseTemplate}; #[tokio::test] async fn the_link_returned_by_subscribe_returns_a_200_if_called() { @@ -17,28 +17,24 @@ async fn the_link_returned_by_subscribe_returns_a_200_if_called() { 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 body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); // Extract the link from one of the request fields. let get_link = |s: &str| { let links: Vec<_> = linkify::LinkFinder::new() .links(s) - .filter(|l| *l.kind() == linkify::LinkKind::Url) + .filter(|l| *l.kind() == linkify::LinkKind::Url) .collect(); assert_eq!(links.len(), 1); links[0].as_str().to_owned() }; let raw_confirmation_link = &get_link(body["HtmlBody"].as_str().unwrap()); let mut confirmation_link = Url::parse(raw_confirmation_link).unwrap(); + assert_eq!(confirmation_link.host_str().unwrap(), "127.0.0.1"); confirmation_link.set_port(Some(app.port)).unwrap(); - println!("\n################################\n{}\n##########################################", confirmation_link); - // Act - let response = reqwest::get(confirmation_link) - .await - .unwrap(); + let response = reqwest::get(confirmation_link).await.unwrap(); // Assert assert_eq!(response.status().as_u16(), 200); @@ -57,4 +53,3 @@ async fn confirmations_without_token_are_rejected_with_a_400() { // Assert assert_eq!(response.status().as_u16(), 400); } - From efd1137c711f034a717ad7c6ff8e51b9f7a42119 Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Thu, 18 Apr 2024 23:24:11 +0200 Subject: [PATCH 09/10] docs: clean up repo and add doc strings --- src/configuration.rs | 7 +++++++ src/routes/subscriptions_confirm.rs | 9 ++++++++- src/startup.rs | 6 ++---- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/configuration.rs b/src/configuration.rs index 4f197dc..a496a3c 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -19,6 +19,12 @@ pub struct Settings { } #[derive(serde::Deserialize, Clone)] +/// The email client settings. +/// +/// * `base_url`: The base URL for the email client +/// * `sender_email`: The email address of the sender +/// * `authorization_token`: The authorization token +/// * `timeout_milliseconds`: The timeout in milliseconds pub struct EmailClientSettings { pub base_url: String, pub sender_email: String, @@ -31,6 +37,7 @@ pub struct EmailClientSettings { /// /// * `port`: The port to listen on /// * `host`: The host address to listen on +/// * `base_url`: The base URL for the application pub struct ApplicationSettings { #[serde(deserialize_with = "deserialize_number_from_string")] pub port: u16, diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs index ca5ed06..613ee13 100644 --- a/src/routes/subscriptions_confirm.rs +++ b/src/routes/subscriptions_confirm.rs @@ -12,12 +12,19 @@ pub struct Parameters { subscription_token: String, } -// #[tracing::instrument(name = "Confirm a pending subscriber", skip(_parameters))] +#[tracing::instrument(name = "Confirm a pending subscriber", skip(_parameters))] +/// Confirm a pending subscriber. +/// # Parameters +/// - subscription_token: The subscription token. +/// # Returns +/// - 200 OK: The subscriber has been confirmed. +/// - 400 Bad Request: The subscription token is missing. pub async fn confirm(Query(_parameters): Query) -> Response { println!("subscription_token: {}", _parameters.subscription_token); (StatusCode::OK,).into_response() } +/// The routes for subscription confirmation. pub fn routes_subscriptions_confirm() -> Router { Router::new().route("/subscriptions/confirm", get(confirm)) } diff --git a/src/startup.rs b/src/startup.rs index 93397c0..dde2937 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -63,15 +63,13 @@ impl Application { ); let listener = TcpListener::bind(&address).await?; - // FIXME: don't clone if not necessary let state = AppState { - db_pool: connection_pool.clone(), - email_client: email_client.clone(), + db_pool: connection_pool, + email_client, base_url: ApplicationBaseUrl(configuration.application.base_url), }; let app = Router::new() - // TODO: check whether state cloning is what we want .merge(crate::routes::routes_subscriptions()) .merge(crate::routes::routes_subscriptions_confirm()) .with_state(state) From 6e07d9c33a522dbd74ab05d997327171aaefd6b2 Mon Sep 17 00:00:00 2001 From: Sandro Eiler Date: Thu, 18 Apr 2024 23:48:34 +0200 Subject: [PATCH 10/10] test: move logic to helpers --- tests/api/helpers.rs | 34 ++++++++++++++++++++++++++++++ tests/api/subscriptions.rs | 17 ++------------- tests/api/subscriptions_confirm.rs | 18 ++-------------- 3 files changed, 38 insertions(+), 31 deletions(-) diff --git a/tests/api/helpers.rs b/tests/api/helpers.rs index a7c598e..6d230b8 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -23,6 +23,12 @@ static TRACING: Lazy<()> = Lazy::new(|| { }; }); +/// Confirmation links embedded in the request to the email API. +pub struct ConfirmationLinks { + pub html: reqwest::Url, + pub plain_text: reqwest::Url, +} + pub struct TestApp { pub address: String, pub port: u16, @@ -47,6 +53,28 @@ impl TestApp { .await .expect("Failed to execute request.") } + pub fn get_confirmation_links(&self, email_request: &wiremock::Request) -> ConfirmationLinks { + let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); + + // Extract the link from one of the request fields. + 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 + }; + + let html = get_link(&body["HtmlBody"].as_str().unwrap()); + let plain_text = get_link(&body["TextBody"].as_str().unwrap()); + ConfirmationLinks { html, plain_text } + } } pub async fn spawn_app() -> TestApp { @@ -93,6 +121,12 @@ pub async fn spawn_app() -> TestApp { } } +/// Create a new database and run the migrations. +/// +/// # Parameters +/// * `config`: The database configuration. +/// # Returns +/// The connection pool. async fn configure_database(config: &DatabaseSettings) -> PgPool { // Create database let mut connection = PgConnection::connect_with(&config.without_db()) diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 7643d7c..2321645 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -18,23 +18,10 @@ async fn subscribe_sends_a_confirmation_email_with_a_link() { app.post_subscriptions(body.into()).await; // Assert - // Get the first intercepted request let email_request = &app.email_server.received_requests().await.unwrap()[0]; - // Parse the body as JSON, starting from raw bytes - let body: serde_json::Value = serde_json::from_slice(&email_request.body).unwrap(); - // Extract the link from one of the request fields. - 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 confirmation_links = app.get_confirmation_links(email_request); // The two links should be identical - assert_eq!(html_link, text_link); + assert_eq!(confirmation_links.html, confirmation_links.plain_text); } #[tokio::test] diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs index 3ed4cfd..e339bf9 100644 --- a/tests/api/subscriptions_confirm.rs +++ b/tests/api/subscriptions_confirm.rs @@ -17,24 +17,10 @@ async fn the_link_returned_by_subscribe_returns_a_200_if_called() { 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(); - // Extract the link from one of the request fields. - 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 raw_confirmation_link = &get_link(body["HtmlBody"].as_str().unwrap()); - let mut confirmation_link = Url::parse(raw_confirmation_link).unwrap(); - - assert_eq!(confirmation_link.host_str().unwrap(), "127.0.0.1"); - confirmation_link.set_port(Some(app.port)).unwrap(); + let confirmation_links = app.get_confirmation_links(email_request); // Act - let response = reqwest::get(confirmation_link).await.unwrap(); + let response = reqwest::get(confirmation_links.html).await.unwrap(); // Assert assert_eq!(response.status().as_u16(), 200);