diff --git a/Cargo.lock b/Cargo.lock index 2cf14cb..14bb877 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1012,16 +1012,6 @@ 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" @@ -1122,7 +1112,6 @@ dependencies = [ "config", "fake", "hyper 1.2.0", - "linkify", "once_cell", "quickcheck", "quickcheck_macros", @@ -1176,15 +1165,6 @@ 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" @@ -2729,7 +2709,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "50bff7831e19200a85b17131d085c25d7811bc4e186efdaf54bbd132994a88cb" dependencies = [ "form_urlencoded", - "idna 0.4.0", + "idna", "percent-encoding", ] @@ -2751,12 +2731,12 @@ dependencies = [ [[package]] name = "validator" -version = "0.17.0" +version = "0.16.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da339118f018cc70ebf01fafc103360528aad53717e4bf311db929cb01cb9345" +checksum = "b92f40481c04ff1f4f61f304d61793c7b56ff76ac1469f1beb199b1445b253bd" dependencies = [ - "idna 0.5.0", - "once_cell", + "idna", + "lazy_static", "regex", "serde", "serde_derive", diff --git a/Cargo.toml b/Cargo.toml index 004b0f3..5c621aa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,10 +19,12 @@ 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"] } @@ -34,7 +36,10 @@ tracing-log = "0.2" secrecy = { version = "0.8", features = ["serde"] } unicode-segmentation = "1" strum_macros = "0.26" -validator = "0.17" +validator = "0.16" + +# async-trait = "0.1" +# strum_macros = "0.25" [dependencies.sqlx] version = "0.7" @@ -63,4 +68,3 @@ quickcheck_macros = "1.0.0" rand = "0.8.5" wiremock = "0.6.0" serde_json = "1" -linkify = "0.10" diff --git a/README.md b/README.md index 414656e..e1b3b2c 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,6 @@ 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 d7f541e..8fd67fa 100644 --- a/configuration/local.yaml +++ b/configuration/local.yaml @@ -1,5 +1,4 @@ application: host: 127.0.0.1 - base_url: "http://127.0.0.1" database: require_ssl: false diff --git a/migrations/20240304202111_create_subscription_tokens_table.sql b/migrations/20240304202111_create_subscription_tokens_table.sql deleted file mode 100644 index 60e260e..0000000 --- a/migrations/20240304202111_create_subscription_tokens_table.sql +++ /dev/null @@ -1,6 +0,0 @@ --- 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) -); diff --git a/src/configuration.rs b/src/configuration.rs index a496a3c..6c8fab1 100644 --- a/src/configuration.rs +++ b/src/configuration.rs @@ -19,12 +19,6 @@ 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, @@ -37,12 +31,10 @@ 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, pub host: String, - pub base_url: String, } #[derive(serde::Deserialize, Clone)] diff --git a/src/domain/subscriber_email.rs b/src/domain/subscriber_email.rs index d6d249d..f2ea9c2 100644 --- a/src/domain/subscriber_email.rs +++ b/src/domain/subscriber_email.rs @@ -1,25 +1,18 @@ -use validator::ValidateEmail; +use validator::validate_email; #[derive(Debug, Clone)] pub struct SubscriberEmail(String); impl SubscriberEmail { pub fn parse(s: String) -> Result { - let result = Self(s.clone()); - if result.validate_email() { - Ok(result) + if validate_email(&s) { + Ok(Self(s)) } 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 diff --git a/src/routes/mod.rs b/src/routes/mod.rs index d0ddba0..90ffeed 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -1,7 +1,5 @@ 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 e276b1b..e59b786 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -1,6 +1,5 @@ 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; @@ -31,37 +30,9 @@ impl TryFrom for NewSubscriber { } } -#[tracing::instrument( - name = "Send a confirmation email to a 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 = format!( - "{}/subscriptions/confirm?subscription_token=mytoken", - base_url - ); - 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", - skip(form, db_pool, email_client, base_url), + skip(form, db_pool, email_client), fields( request_id = %Uuid::new_v4(), subscriber_email = %form.email, @@ -72,7 +43,6 @@ pub async fn subscribe( State(AppState { db_pool, email_client, - base_url, }): State, Form(form): Form, ) -> Response { @@ -82,16 +52,14 @@ pub async fn subscribe( return (StatusCode::BAD_REQUEST, "Invalid subscription.").into_response(); } }; - if insert_subscriber(&db_pool, &new_subscriber).await.is_err() { - return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").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 send_confirmation_email(&email_client, new_subscriber, &base_url.0) - .await - .is_err() - { - return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong.").into_response(); - } - return (StatusCode::OK,).into_response(); } #[tracing::instrument( @@ -105,13 +73,15 @@ pub async fn insert_subscriber( let _ = sqlx::query!( r#" INSERT INTO subscriptions (id, email, name, subscribed_at, status) - VALUES ($1, $2, $3, $4, 'pending_confirmation') + VALUES ($1, $2, $3, $4, 'confirmed') "#, 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| { @@ -121,6 +91,8 @@ pub async fn insert_subscriber( Ok(()) } -pub fn routes_subscriptions() -> Router { - Router::new().route("/subscriptions", post(subscribe)) +pub fn routes_subscriptions(state: AppState) -> Router { + Router::new() + .route("/subscriptions", post(subscribe)) + .with_state(state) } diff --git a/src/routes/subscriptions_confirm.rs b/src/routes/subscriptions_confirm.rs deleted file mode 100644 index 613ee13..0000000 --- a/src/routes/subscriptions_confirm.rs +++ /dev/null @@ -1,30 +0,0 @@ -use crate::startup::AppState; -use axum::routing::get; -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))] -/// 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 dde2937..c491757 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -21,14 +21,10 @@ 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 @@ -64,16 +60,12 @@ impl Application { let listener = TcpListener::bind(&address).await?; let state = AppState { - db_pool: connection_pool, - email_client, - base_url: ApplicationBaseUrl(configuration.application.base_url), + db_pool: connection_pool.clone(), + email_client: email_client.clone(), }; - let app = Router::new() - .merge(crate::routes::routes_subscriptions()) - .merge(crate::routes::routes_subscriptions_confirm()) - .with_state(state) .merge(crate::routes::routes_health_check()) + .merge(crate::routes::routes_subscriptions(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 6d230b8..ccee1ce 100644 --- a/tests/api/helpers.rs +++ b/tests/api/helpers.rs @@ -4,7 +4,6 @@ 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(|| { @@ -23,17 +22,9 @@ 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, pub db_pool: PgPool, - pub email_server: MockServer, } impl TestApp { @@ -53,28 +44,6 @@ 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 { @@ -82,8 +51,9 @@ pub async fn spawn_app() -> TestApp { // All other invocations will instead skip execution. Lazy::force(&TRACING); - // Launch a mock server to stand in for Postmark's API - let email_server = MockServer::start().await; + // TODO: + // // 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 = { @@ -92,8 +62,6 @@ 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 }; @@ -104,29 +72,22 @@ pub async fn spawn_app() -> TestApp { .await .expect("Failed to build application."); // Get the port before spawning the application - let application_port = application.port(); - let address = format!("http://127.0.0.1:{}", 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, + // email_server, // test_user: TestUser::generate(), // api_client: client, // email_client: configuration.email_client.client(), } } -/// 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/main.rs b/tests/api/main.rs index 177847a..3b9c227 100644 --- a/tests/api/main.rs +++ b/tests/api/main.rs @@ -1,4 +1,3 @@ mod health_check; mod helpers; mod subscriptions; -mod subscriptions_confirm; diff --git a/tests/api/subscriptions.rs b/tests/api/subscriptions.rs index 2321645..276fd09 100644 --- a/tests/api/subscriptions.rs +++ b/tests/api/subscriptions.rs @@ -1,48 +1,4 @@ -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 - let email_request = &app.email_server.received_requests().await.unwrap()[0]; - let confirmation_links = app.get_confirmation_links(email_request); - // The two links should be identical - assert_eq!(confirmation_links.html, confirmation_links.plain_text); -} - -#[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 -} +use crate::helpers::{spawn_app, TestApp}; #[tokio::test] async fn subscribe_returns_a_422_when_data_is_missing() { @@ -73,42 +29,20 @@ 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; // Assert assert_eq!(200, response.status().as_u16()); -} -#[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",) + let saved = sqlx::query!("SELECT email, name 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] @@ -129,7 +63,7 @@ async fn subscribe_returns_a_400_when_fields_are_present_but_invalid() { assert_eq!( 400, response.status().as_u16(), - "The API did not fail with 400 when the payload was {}.", + "The API did not return a 200 OK when the payload was {}.", description ); } diff --git a/tests/api/subscriptions_confirm.rs b/tests/api/subscriptions_confirm.rs deleted file mode 100644 index e339bf9..0000000 --- a/tests/api/subscriptions_confirm.rs +++ /dev/null @@ -1,41 +0,0 @@ -use crate::helpers::spawn_app; -use reqwest::Url; -use wiremock::matchers::{method, path}; -use wiremock::{Mock, ResponseTemplate}; - -#[tokio::test] -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 confirmation_links = app.get_confirmation_links(email_request); - - // Act - let response = reqwest::get(confirmation_links.html).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; - - // Act - let response = reqwest::get(&format!("{}/subscriptions/confirm", app.address)) - .await - .unwrap(); - - // Assert - assert_eq!(response.status().as_u16(), 400); -}