diff --git a/Cargo.lock b/Cargo.lock index 155ef36..200af0d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -245,6 +245,15 @@ dependencies = [ "windows-targets 0.48.1", ] +[[package]] +name = "claims" +version = "0.7.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6995bbe186456c36307f8ea36be3eefe42f49d106896414e18efc4fb2f846b5" +dependencies = [ + "autocfg", +] + [[package]] name = "config" version = "0.14.0" @@ -395,6 +404,12 @@ dependencies = [ "powerfmt", ] +[[package]] +name = "deunicode" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ae2a35373c5c74340b79ae6780b498b2b183915ec5dacf263aac5a099bf485a" + [[package]] name = "digest" version = "0.10.7" @@ -440,6 +455,16 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "env_logger" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a19187fea3ac7e84da7dacf48de0c45d63c6a76f9490dae389aead16c243fce3" +dependencies = [ + "log", + "regex", +] + [[package]] name = "equivalent" version = "1.0.1" @@ -484,6 +509,16 @@ version = "2.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0206175f82b8d6bf6652ff7d71a1e27fd2e4efde587fd368662814d6ec1d9ce0" +[[package]] +name = "fake" +version = "2.9.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c25829bde82205da46e1823b2259db6273379f626fc211f126f65654a2669be" +dependencies = [ + "deunicode", + "rand", +] + [[package]] name = "fastrand" version = "1.9.0" @@ -1040,9 +1075,14 @@ version = "0.2.0" dependencies = [ "axum", "chrono", + "claims", "config", + "fake", "hyper 1.1.0", "once_cell", + "quickcheck", + "quickcheck_macros", + "rand", "reqwest", "secrecy", "serde", @@ -1059,6 +1099,7 @@ dependencies = [ "tracing-subscriber", "unicode-segmentation", "uuid", + "validator", ] [[package]] @@ -1538,6 +1579,28 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "quickcheck" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "588f6378e4dd99458b60ec275b4477add41ce4fa9f64dcba6f15adccb19b50d6" +dependencies = [ + "env_logger", + "log", + "rand", +] + +[[package]] +name = "quickcheck_macros" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b22a693222d716a9587786f37ac3f6b4faedb5b80c23914e7303ff5a1d8016e9" +dependencies = [ + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "quote" version = "1.0.29" @@ -2723,6 +2786,21 @@ dependencies = [ "rand", ] +[[package]] +name = "validator" +version = "0.16.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b92f40481c04ff1f4f61f304d61793c7b56ff76ac1469f1beb199b1445b253bd" +dependencies = [ + "idna", + "lazy_static", + "regex", + "serde", + "serde_derive", + "serde_json", + "url", +] + [[package]] name = "valuable" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index ffc37ed..75be7c3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,8 @@ tracing-log = "0.2" secrecy = { version = "0.8", features = ["serde"] } unicode-segmentation = "1" strum_macros = "0.26" +validator = "0.16" + # async-trait = "0.1" # strum_macros = "0.25" @@ -54,3 +56,8 @@ features = [ [dev-dependencies] reqwest = "0.11" once_cell = "1" +claims = "0.7" +fake = "2.9.2" +quickcheck = "1.0.3" +quickcheck_macros = "1.0.0" +rand = "0.8.5" diff --git a/src/domain/mod.rs b/src/domain/mod.rs new file mode 100644 index 0000000..e771a2e --- /dev/null +++ b/src/domain/mod.rs @@ -0,0 +1,7 @@ +mod new_subscriber; +mod subscriber_email; +mod subscriber_name; + +pub use new_subscriber::NewSubscriber; +pub use subscriber_email::SubscriberEmail; +pub use subscriber_name::SubscriberName; diff --git a/src/domain/new_subscriber.rs b/src/domain/new_subscriber.rs new file mode 100644 index 0000000..dad8a71 --- /dev/null +++ b/src/domain/new_subscriber.rs @@ -0,0 +1,7 @@ +use crate::domain::subscriber_email::SubscriberEmail; +use crate::domain::subscriber_name::SubscriberName; + +pub struct NewSubscriber { + pub email: SubscriberEmail, + pub name: SubscriberName, +} diff --git a/src/domain/subscriber_email.rs b/src/domain/subscriber_email.rs new file mode 100644 index 0000000..e28d153 --- /dev/null +++ b/src/domain/subscriber_email.rs @@ -0,0 +1,76 @@ +use validator::validate_email; + +#[derive(Debug)] +pub struct SubscriberEmail(String); + +impl SubscriberEmail { + pub fn parse(s: String) -> Result { + if validate_email(&s) { + Ok(Self(s)) + } else { + Err(format!("{} is not a valid subscriber email.", s)) + } + } +} + +impl AsRef for SubscriberEmail { + fn as_ref(&self) -> &str { + &self.0 + } +} + +#[cfg(test)] +mod tests { + use super::SubscriberEmail; + use claims::assert_err; + use fake::faker::internet::en::SafeEmail; + use fake::Fake; + use rand::{rngs::StdRng, SeedableRng}; + + #[derive(Debug, Clone)] + struct ValidEmailFixture(pub String); + + impl quickcheck::Arbitrary for ValidEmailFixture { + fn arbitrary(g: &mut quickcheck::Gen) -> Self { + let mut rng = StdRng::seed_from_u64(u64::arbitrary(g)); + let email = SafeEmail().fake_with_rng(&mut rng); + Self(email) + } + } + + #[quickcheck_macros::quickcheck] + fn valid_emails_are_parsed_successfully(valid_email: ValidEmailFixture) -> bool { + dbg!(&valid_email.0); + SubscriberEmail::parse(valid_email.0).is_ok() + } + + #[test] + fn empty_string_is_rejected() { + let email = "".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } + + #[test] + fn email_missing_at_symbol_is_rejected() { + let email = "ursuladomain.com".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } + + #[test] + fn email_missing_subject_is_rejected() { + let email = "@domain.com".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } + + #[test] + fn email_sharp_s_prefix_is_rejected() { + let email = "joe.ßchmidt@example.com".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } + + #[test] + fn email_with_invalid_characters_is_rejected() { + let email = "joe.smith@ex ample.com".to_string(); + assert_err!(SubscriberEmail::parse(email)); + } +} diff --git a/src/domain/subscriber_name.rs b/src/domain/subscriber_name.rs new file mode 100644 index 0000000..5c7ec4e --- /dev/null +++ b/src/domain/subscriber_name.rs @@ -0,0 +1,70 @@ +use unicode_segmentation::UnicodeSegmentation; +#[derive(Debug)] +pub struct SubscriberName(String); +impl SubscriberName { + /// Returns an instance of `SubscriberName` if the input satisfies all + /// our validation constraints on subscriber names. + pub fn parse(s: String) -> Result { + let is_empty_or_whitespace = s.trim().is_empty(); + + let is_too_long = s.graphemes(true).count() > 256; + + let forbidden_characters = ['/', '(', ')', '"', '<', '>', '\\', '{', '}']; + let contains_forbidden_characters = s.chars().any(|g| forbidden_characters.contains(&g)); + + if is_empty_or_whitespace || is_too_long || contains_forbidden_characters { + Err(format!("{} is not a valid subscriber name.", s)) + } else { + Ok(Self(s)) + } + } +} +impl AsRef for SubscriberName { + fn as_ref(&self) -> &str { + &self.0 + } +} + +#[cfg(test)] +mod tests { + use crate::domain::SubscriberName; + use claims::{assert_err, assert_ok}; + + #[test] + fn a_256_grapheme_long_name_is_valid() { + let name = "ё".repeat(256); + assert_ok!(SubscriberName::parse(name)); + } + + #[test] + fn a_name_longer_than_256_graphemes_is_rejected() { + let name = "a".repeat(257); + assert_err!(SubscriberName::parse(name)); + } + + #[test] + fn whitespace_only_names_are_rejected() { + let name = " ".to_string(); + assert_err!(SubscriberName::parse(name)); + } + + #[test] + fn empty_string_is_rejected() { + let name = "".to_string(); + assert_err!(SubscriberName::parse(name)); + } + + #[test] + fn names_containing_an_invalid_character_are_rejected() { + for name in &['/', '(', ')', '"', '<', '>', '\\', '{', '}'] { + let name = name.to_string(); + assert_err!(SubscriberName::parse(name)); + } + } + + #[test] + fn a_valid_name_is_parsed_successfully() { + let name = "Ursula Le Guin".to_string(); + assert_ok!(SubscriberName::parse(name)); + } +} diff --git a/src/error.rs b/src/error.rs deleted file mode 100644 index 349a020..0000000 --- a/src/error.rs +++ /dev/null @@ -1,78 +0,0 @@ -use axum::http::StatusCode; -use axum::response::{IntoResponse, Response}; -use serde::Serialize; - -pub type Result = core::result::Result; - -#[derive(Clone, Debug, Serialize, strum_macros::AsRefStr)] -#[serde(tag = "type", content = "data")] -pub enum Error { - LoginFail, - - // -- Auth errors. - AuthFailNoAuthTokenCookie, - AuthFailTokenWrongFormat, - AuthFailCtxNotInRequestExt, - - // -- Model errors. - // FIXME: Delete this: - PropertyDeleteFailIdNotFound { id: u64 }, - - // -- Service errors. - SubscriptionFailInvalidName, -} - -impl Error { - pub fn client_status_and_error(&self) -> (StatusCode, ClientError) { - match self { - // -- Login. - Self::LoginFail => (StatusCode::UNAUTHORIZED, ClientError::LOGIN_FAIL), - - // -- Auth. - Self::AuthFailNoAuthTokenCookie - | Self::AuthFailTokenWrongFormat - | Self::AuthFailCtxNotInRequestExt => (StatusCode::FORBIDDEN, ClientError::NO_AUTH), - - // -- Model. - // FIXME: Delete this: - Self::PropertyDeleteFailIdNotFound { .. } => { - (StatusCode::BAD_REQUEST, ClientError::INVALID_PARAMS) - } - - // -- Subscription. - Self::SubscriptionFailInvalidName => { - (StatusCode::BAD_REQUEST, ClientError::INVALID_PARAMS) - } - - // -- Fallback. - #[allow(unreachable_patterns)] - _ => ( - StatusCode::INTERNAL_SERVER_ERROR, - ClientError::SERVICE_ERROR, - ), - } - } -} - -impl IntoResponse for Error { - fn into_response(self) -> Response { - // TODO: trace something here maybe. - - // Create a placeholder Axum response. - let mut response = StatusCode::INTERNAL_SERVER_ERROR.into_response(); - - // Insert the Error into the response. - response.extensions_mut().insert(self); - - response - } -} - -#[derive(Debug, strum_macros::AsRefStr)] -#[allow(non_camel_case_types)] -pub enum ClientError { - LOGIN_FAIL, - NO_AUTH, - INVALID_PARAMS, - SERVICE_ERROR, -} diff --git a/src/lib.rs b/src/lib.rs index e8899a6..f948e62 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,10 +2,8 @@ //! //! This is an example documentation string for the root of the crate. -pub use self::error::{Error, Result}; - pub mod configuration; -pub mod error; +pub mod domain; pub mod routes; pub mod startup; pub mod telemetry; diff --git a/src/routes/subscriptions.rs b/src/routes/subscriptions.rs index 86b6535..b1ea5e4 100644 --- a/src/routes/subscriptions.rs +++ b/src/routes/subscriptions.rs @@ -1,4 +1,5 @@ -use crate::Result; +use crate::domain::SubscriberEmail; +use crate::domain::{NewSubscriber, SubscriberName}; use axum::extract::State; use axum::routing::post; use axum::Form; @@ -10,7 +11,6 @@ use axum::{ use chrono::Utc; use serde::Deserialize; use sqlx::PgPool; -use unicode_segmentation::UnicodeSegmentation; use uuid::Uuid; #[derive(Debug, Deserialize)] @@ -19,32 +19,6 @@ struct FormData { name: String, } -/// Returns `true` if the input satisfies all our validation constraints -/// on subscriber names, `false` otherwise. -pub fn is_valid_name(s: &str) -> bool { - // `.trim()` returns a view over the input `s` without trailing - // whitespace-like characters. - // `.is_empty` checks if the view contains any character. - let is_empty_or_whitespace = s.trim().is_empty(); - - // A grapheme is defined by the Unicode standard as a "user-perceived" - // character: `å` is a single grapheme, but it is composed of two characters - // (`a` and `̊`). - // - // `graphemes` returns an iterator over the graphemes in the input `s`. - // `true` specifies that we want to use the extended grapheme definition set, - // the recommended one. - let is_too_long = s.graphemes(true).count() > 256; - - // Iterate over all characters in the input `s` to check if any of them matches - // one of the characters in the forbidden array. - let forbidden_characters = ['/', '(', ')', '"', '<', '>', '\\', '{', '}']; - let contains_forbidden_characters = s.chars().any(|g| forbidden_characters.contains(&g)); - - // Return `false` if any of our conditions have been violated - !(is_empty_or_whitespace || is_too_long || contains_forbidden_characters) -} - #[tracing::instrument( name = "Adding a new subscriber", skip(form, pool), @@ -55,17 +29,24 @@ pub fn is_valid_name(s: &str) -> bool { ) )] pub async fn subscribe(State(pool): State, Form(form): Form) -> Response { - if !is_valid_name(&form.name) { - tracing::error!("Failed to add subscriber to the database"); - return (StatusCode::BAD_REQUEST, "Invalid name").into_response(); - } - match insert_subscriber(&pool, &form).await { + let name = match SubscriberName::parse(form.name) { + Ok(name) => name, + Err(_) => { + return (StatusCode::BAD_REQUEST, "Invalid name").into_response(); + } + }; + let email = match SubscriberEmail::parse(form.email) { + Ok(email) => email, + Err(_) => { + return (StatusCode::BAD_REQUEST, "Invalid email address").into_response(); + } + }; + let new_subscriber = NewSubscriber { email, name }; + match insert_subscriber(&pool, &new_subscriber).await { Ok(_) => { - tracing::info!("Subscriber added to the database"); return (StatusCode::OK,).into_response(); } Err(_) => { - tracing::error!("Failed to add subscriber to the database"); return (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong").into_response(); } } @@ -73,17 +54,20 @@ pub async fn subscribe(State(pool): State, Form(form): Form) - #[tracing::instrument( name = "Saving new subscriber details in the database", - skip(form, pool) + skip(new_subscriber, pool) )] -pub async fn insert_subscriber(pool: &PgPool, form: &FormData) -> Result<()> { +pub async fn insert_subscriber( + pool: &PgPool, + new_subscriber: &NewSubscriber, +) -> Result<(), sqlx::Error> { let _ = sqlx::query!( r#" INSERT INTO subscriptions (id, email, name, subscribed_at) VALUES ($1, $2, $3, $4) "#, Uuid::new_v4(), - form.email, - form.name, + new_subscriber.email.as_ref(), + new_subscriber.name.as_ref(), Utc::now() ) // We use `get_ref` to get an immutable reference to the `PgConnection` diff --git a/tests/health_check.rs b/tests/health_check.rs index 5103251..079df55 100644 --- a/tests/health_check.rs +++ b/tests/health_check.rs @@ -105,7 +105,7 @@ async fn subscribe_returns_a_200_for_valid_form_data() { } #[tokio::test] -async fn subscribe_returns_a_200_when_fields_are_present_but_empty() { +async fn subscribe_returns_a_400_when_fields_are_present_but_invalid() { // Arrange let app = spawn_app().await; let client = reqwest::Client::new(); @@ -127,7 +127,7 @@ async fn subscribe_returns_a_200_when_fields_are_present_but_empty() { // Assert assert_eq!( - 200, + 400, response.status().as_u16(), "The API did not return a 200 OK when the payload was {}.", description