chore: Tested duplicated subscription

Author: Utilitycoder

Cargo.lock

@@ -17,7 +17,7 @@ actix-web = "4"
 tokio = { version = "1", features = ["macros", "rt-multi-thread"] }
 serde = {version = "1.0.163", features = ["derive"]}
 serde-aux = "4.2.0" 
-config = "0.11"
+config = {version = "0.13", default-features = false, features = ["yaml"] }
 uuid = { version = "1.3.3", features = ["v4"] }
 chrono = "0.4.15"
 tracing = { version = "0.1", features = ["log"] }
@@ -26,7 +26,7 @@ tracing-bunyan-formatter = "0.3.3"
 tracing-log = "0.1"
 secrecy = { version = "0.8", features = ["serde"] }
 tracing-actix-web = "0.5"
-sqlx = { version = "0.5.7", default-features = false, features = ["offline", "runtime-actix-rustls", "macros", "postgres", "uuid", "chrono", "migrate"] }
+sqlx = { version = "0.6", default-features = false, features = ["runtime-actix-rustls", "macros", "postgres", "uuid", "chrono", "migrate", "offline"] }
 unicode-segmentation = "1"
 validator = "0.16.0"
 reqwest = { version = "0.11.18", default-features = false, features = ["json", "rustls-tls"] }

Cargo.toml

@@ -65,4 +65,4 @@ databases:
   num_nodes: 1
   size: db-s-dev-database
   # Postgres version - using the latest here
-  version: "12"
+  version: "14"

spec.yaml

@@ -1,18 +1,3 @@
 {
-  "db": "PostgreSQL",
-  "bcfcfebc6f5e8ffbf97d97c5a209be78b46d703924482cf8b43842705fcb7714": {
-    "describe": {
-      "columns": [],
-      "nullable": [],
-      "parameters": {
-        "Left": [
-          "Uuid",
-          "Text",
-          "Text",
-          "Timestamptz"
-        ]
-      }
-    },
-    "query": "\n        INSERT INTO subscriptions (id, email, name, subscribed_at)\n        VALUES ($1, $2, $3, $4)\n        "
-  }
+  "db": "PostgreSQL"
 }

sqlx-data.json

@@ -3,6 +3,7 @@ use secrecy::{ExposeSecret, Secret};
 use serde_aux::field_attributes::deserialize_number_from_string;
 use sqlx::postgres::{PgConnectOptions, PgSslMode};
 use sqlx::ConnectOptions;
+use std::convert::{TryFrom, TryInto};
 use tracing_log::log;
 
 #[derive(serde::Deserialize, Clone)]
@@ -73,28 +74,27 @@ impl DatabaseSettings {
 
 pub fn get_configuration() -> Result<Settings, config::ConfigError> {
     // Initialize our configuration reader
-    let mut settings = config::Config::default();
     let base_path = std::env::current_dir().expect("Failed to determine the current directory");
     let configuration_directory = base_path.join("configuration");
 
-    // Read the "default" configuration file
-    settings.merge(config::File::from(configuration_directory.join("base")).required(true))?;
-
     // Detect the running environment
     // Default to 'local' if unspecified
     let environment: Environment = std::env::var("APP_ENVIRONMENT")
         .unwrap_or_else(|_| "local".into())
         .try_into()
         .expect("APP_ENVIRONMENT must be 'local' or 'production'");
 
-    // Layer on environment specific values
-    settings.merge(
-        config::File::from(configuration_directory.join(environment.as_str())).required(true),
-    )?;
+    let environment_filename = format!("{}.yml", environment.as_str());
 
-    settings.merge(config::Environment::with_prefix("app").separator("__"))?;
+    let settings = config::Config::builder().add_source(
+        config::File::from(configuration_directory.join("base"),
+    )).add_source(
+        config::File::from(configuration_directory.join(environment_filename)),
+    ).add_source(
+        config::Environment::with_prefix("app").prefix_separator("_").separator("__"),
+    ).build()?;
 
-    settings.try_into()
+    settings.try_deserialize::<Settings>()
 }
 
 // The possible runtime environment for our application
@@ -105,7 +105,7 @@ pub enum Environment {
 }
 
 impl Environment {
-    pub fn as_str(&self) -> &str {
+    pub fn as_str(&self) -> &'static str {
         match self {
             Environment::Local => "local",
             Environment::Production => "production",

src/configuration.rs

@@ -1,6 +1,6 @@
 use validator::validate_email;
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone)]
 pub struct SubscriberEmail(String);
 
 impl SubscriberEmail {

src/domain/subscriber_email.rs

@@ -44,9 +44,9 @@ impl EmailClient {
             text_body: text_content,
         };
 
-        let _builder = self
+        self
             .http_client
-            .post(url)
+            .post(&url)
             .header(
                 "X-Postmark-Server-Token",
                 self.authorization_token.expose_secret(),
@@ -62,11 +62,11 @@ impl EmailClient {
 #[derive(serde::Serialize)]
 #[serde(rename_all = "PascalCase")]
 pub struct SendEmailRequest<'a> {
-    pub from: &'a str,
-    pub to: &'a str,
-    pub subject: &'a str,
-    pub html_body: &'a str,
-    pub text_body: &'a str,
+    from: &'a str,
+    to: &'a str,
+    subject: &'a str,
+    html_body: &'a str,
+    text_body: &'a str,
 }
 
 #[cfg(test)]

src/email_client.rs

@@ -1,5 +1,5 @@
 use actix_web::HttpResponse;
 
 pub async fn health_check() -> HttpResponse {
-    HttpResponse::Ok().into()
+    HttpResponse::Ok().finish()
 }

src/routes/health_check.rs

@@ -6,9 +6,9 @@ use crate::{
 use actix_web::{web, HttpResponse};
 use chrono::Utc;
 use rand::{distributions::Alphanumeric, thread_rng, Rng};
-
-use sqlx::types::Uuid;
 use sqlx::{PgPool, Postgres, Transaction};
+use std::convert::{TryInto, TryFrom};
+use uuid::Uuid;
 
 #[allow(dead_code)]
 #[derive(serde::Deserialize)]
@@ -27,12 +27,13 @@ impl TryFrom<FormData> for NewSubscriber {
     }
 }
 
+#[allow(clippy::async_yields_async)]
 #[tracing::instrument(
     name = "Adding a new subscriber",
     skip(form, pool, email_client, base_url),
     fields(
-        email = %form.email,
-        name = %form.name
+        subscriber_email = %form.email,
+        subscriber_name = %form.name
     )
 )]
 pub async fn subscribe(
@@ -43,22 +44,30 @@ pub async fn subscribe(
 ) -> HttpResponse {
     let new_subscriber = match form.0.try_into() {
         Ok(subscriber) => subscriber,
-        Err(e) => return HttpResponse::BadRequest().body(e),
+        Err(_) => return HttpResponse::BadRequest().finish(),
     };
 
     let mut transaction = match pool.begin().await {
         Ok(t) => t,
         Err(_) => return HttpResponse::InternalServerError().finish(),
     };
 
+    if search_for_existing_subscription(&new_subscriber, &mut transaction).await.is_err() {
+        return HttpResponse::InternalServerError().finish();
+    }
+
     let subscriber_id = match insert_subscriber(&new_subscriber, &mut transaction).await {
         Ok(id) => id,
         Err(_) => return HttpResponse::InternalServerError().finish(),
     };
 
     let subscription_token = generate_subscription_token();
 
-    if store_token(subscriber_id, &subscription_token, &pool).await.is_err() {
+    if store_token(subscriber_id, &subscription_token, &mut transaction).await.is_err() {
+        return HttpResponse::InternalServerError().finish();
+    }
+
+    if transaction.commit().await.is_err() {
         return HttpResponse::InternalServerError().finish();
     }
 
@@ -69,10 +78,6 @@ pub async fn subscribe(
         return HttpResponse::InternalServerError().finish();
     }
 
-    if transaction.commit().await.is_err() {
-        return HttpResponse::InternalServerError().finish();
-    }
-
     HttpResponse::Ok().finish()
 }
 
@@ -142,12 +147,12 @@ async fn insert_subscriber(
 /// Store subscription token in the database
 #[tracing::instrument(
     name = "Saving subscription token in the database",
-    skip(subscriber_id, subscription_token, pool)
+    skip(subscriber_id, subscription_token, transaction)
 )]
 async fn store_token(
     subscriber_id: Uuid,
     subscription_token: &str,
-    pool: &PgPool,
+    transaction: &mut Transaction <'_ , Postgres>
 ) -> Result<(), sqlx::Error> {
     sqlx::query!(
         r#"
@@ -157,7 +162,7 @@ async fn store_token(
         subscription_token,
         subscriber_id,
     )
-    .execute(pool)
+    .execute(transaction)
     .await
     .map_err(|e| {
         tracing::error!("Failed to execute query: {:?}", e);
@@ -173,4 +178,26 @@ fn generate_subscription_token() -> String {
         .map(char::from)
         .take(25)
         .collect()
+}
+
+async fn search_for_existing_subscription(
+    new_subscriber: &NewSubscriber,
+    transaction: &mut Transaction<'_, Postgres>
+) -> Result<(), sqlx::Error> {
+    sqlx::query!(
+        r#"
+        SELECT email, name
+        FROM subscriptions
+        WHERE email = $1 AND name = $2
+        "#,
+        new_subscriber.email.as_ref(),
+        new_subscriber.name.as_ref(),
+    )
+    .fetch_optional(transaction)
+    .await
+    .map_err(|e| {
+        tracing::error!("Failed to execute query: {:?}", e);
+        e
+    })?;
+    Ok(())
 }

src/routes/subscriptions.rs

@@ -1,5 +1,6 @@
 use actix_web::{web, HttpResponse};
-use sqlx::{PgPool, types::Uuid};
+use sqlx::PgPool;
+use uuid::Uuid;
 
 #[derive(serde::Deserialize)]
 pub struct Parameters {
@@ -10,7 +11,7 @@ pub struct Parameters {
 pub async fn confirm(parameters: web::Query<Parameters>, pool: web::Data<PgPool>) -> HttpResponse {
     let id = match get_subscriber_id_from_token(&parameters.subscription_token, &pool).await {
         Ok(id) => id,
-        Err(_) => return HttpResponse::BadRequest().finish(),
+        Err(_) => return HttpResponse::InternalServerError().finish(),
     };
 
     match id {

src/routes/subscriptions_confirm.rs

@@ -58,7 +58,7 @@ impl Application {
 
 pub fn get_connection_pool(configuration: &DatabaseSettings) -> PgPool {
     PgPoolOptions::new()
-        .connect_timeout(std::time::Duration::from_secs(2))
+        .acquire_timeout(std::time::Duration::from_secs(2))
         .connect_lazy_with(configuration.with_db())
 }
 
@@ -72,6 +72,7 @@ pub fn run(
 ) -> Result<Server, std::io::Error> {
     let db_pool = web::Data::new(db_pool);
     let email_client = web::Data::new(email_client);
+    let base_url = web::Data::new(ApplicationBaseUrl(base_url));
     let server = HttpServer::new(move || {
         App::new()
             // Middleware logger added here

src/startup.rs

@@ -50,10 +50,10 @@ impl TestApp {
             confirmation_link
         };
 
-        let html = get_link(body["html_body"].as_str().unwrap());
-        let plain_text = get_link(body["plain_text_body"].as_str().unwrap());
-
+        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 post_subscriptions(&self, body: String) -> reqwest::Response {
@@ -77,6 +77,7 @@ pub async fn spawn_app() -> TestApp {
         let mut c = get_configuration().expect("Failed to read configuration.");
         c.database.database_name = Uuid::new_v4().to_string();
         c.application.port = 0; // random free port
+        c.email_client.base_url = mock_server.uri();
         c
     };
 

tests/api/helpers.rs

@@ -17,8 +17,8 @@ async fn subscribe_returns_a_200_for_valid_form_data() {
         .await;
 
     let response = app.post_subscriptions(body.into()).await;
+
     // Assert
-    println!("Response: {:?}", response);
     assert_eq!(200, response.status().as_u16());
 }
 
@@ -45,6 +45,21 @@ async fn subscribe_persists_the_new_subscriber() {
     assert_eq!(saved.status, "pending_confirmation");
 }
 
+#[tokio::test]
+async fn subscribe_sends_aconfirmation_email_for_valid_data() {
+    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.mock_server)
+        .await;
+
+    app.post_subscriptions(body.into()).await;
+}
+
 #[tokio::test]
 async fn subscribe_returns_a_400_when_data_is_missing() {
     // Arrange
@@ -108,18 +123,23 @@ async fn subscribe_sends_a_confirmation_email_with_a_link() {
     app.post_subscriptions(body.into()).await;
 
     let email_request = &app.mock_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_body = body["html_body"].as_str().unwrap();
-    let text_body = body["text_body"].as_str().unwrap();
-    assert_eq!(get_link(html_body), get_link(text_body));
+    let confirmation_link = app.get_confirmation_links(email_request);
+
+    assert_eq!(confirmation_link.html, confirmation_link.plain_text);
+}
+
+// test for when a user tries to subscribe twice
+#[tokio::test]
+async fn subscribe_fails_if_there_is_a_duplicated_email() {
+    let app = spawn_app().await;
+    let body = "name=le%20guin&email=ursula_le_guin%40gmail.com";
+
+    // first attempt is successful
+    app.post_subscriptions(body.into()).await;
+
+    // second attempt fails
+    let response = app.post_subscriptions(body.into()).await;
+    println!("{:?}", response);
+
+    assert_eq!(response.status().as_u16(), 500);
 }