From 845f8b62eddc0523de9d95d19cf25635a31b6fbd Mon Sep 17 00:00:00 2001 From: Nicholas Chitty Date: Sat, 29 Jun 2024 16:23:18 -0400 Subject: [PATCH 1/5] Add mockall Add mockall --- lambda/Cargo.lock | 78 +++++++++++++++++++++++++++++++++++++++++++++++ lambda/Cargo.toml | 1 + 2 files changed, 79 insertions(+) diff --git a/lambda/Cargo.lock b/lambda/Cargo.lock index 9fd412e..1298cf7 100644 --- a/lambda/Cargo.lock +++ b/lambda/Cargo.lock @@ -17,6 +17,12 @@ version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f26201604c87b1e01bd3d98f8d5d9a8fcbb815e8cedb41ffccbeb4bf593a35fe" +[[package]] +name = "anstyle" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "038dfcf04a5feb68e9c60b21c9625a54c2c0616e79b72b0fd87075a056ae1d1b" + [[package]] name = "async-stream" version = "0.3.5" @@ -608,6 +614,12 @@ dependencies = [ "subtle", ] +[[package]] +name = "downcast" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1435fa1053d8b2fbbe9be7e97eca7f33d37b28409959813daefc1446a14247f1" + [[package]] name = "either" version = "1.9.0" @@ -650,6 +662,12 @@ dependencies = [ "percent-encoding", ] +[[package]] +name = "fragile" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c2141d6d6c8512188a7891b4b01590a45f6dac67afb4f255c4124dbb86d4eaa" + [[package]] name = "futures" version = "0.3.28" @@ -1025,6 +1043,7 @@ dependencies = [ "axum", "lambda_http", "lambda_runtime", + "mockall", "serde", "serde_dynamo", "serde_json", @@ -1164,6 +1183,33 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "mockall" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "43766c2b5203b10de348ffe19f7e54564b64f3d6018ff7648d1e2d6d3a0f0a48" +dependencies = [ + "cfg-if", + "downcast", + "fragile", + "lazy_static", + "mockall_derive", + "predicates", + "predicates-tree", +] + +[[package]] +name = "mockall_derive" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "af7cbce79ec385a1d4f54baa90a76401eb15d9cab93685f62e7e9f942aa00ae2" +dependencies = [ + "cfg-if", + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "num-conv" version = "0.1.0" @@ -1276,6 +1322,32 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" +[[package]] +name = "predicates" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "68b87bfd4605926cdfefc1c3b5f8fe560e3feca9d5552cf68c466d3d8236c7e8" +dependencies = [ + "anstyle", + "predicates-core", +] + +[[package]] +name = "predicates-core" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b794032607612e7abeb4db69adb4e33590fa6cf1149e95fd7cb00e634b92f174" + +[[package]] +name = "predicates-tree" +version = "1.0.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "368ba315fb8c5052ab692e68a0eefec6ec57b23a36959c14496f0b0df2c0cecf" +dependencies = [ + "predicates-core", + "termtree", +] + [[package]] name = "proc-macro2" version = "1.0.76" @@ -1615,6 +1687,12 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2047c6ded9c721764247e62cd3b03c09ffc529b2ba5b10ec482ae507a4a70160" +[[package]] +name = "termtree" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3369f5ac52d5eb6ab48c6b4ffdc8efbcad6b89c765749064ba298f2c68a16a76" + [[package]] name = "thread_local" version = "1.1.7" diff --git a/lambda/Cargo.toml b/lambda/Cargo.toml index a424211..c3b95d7 100644 --- a/lambda/Cargo.toml +++ b/lambda/Cargo.toml @@ -12,6 +12,7 @@ aws-sdk-secretsmanager = "1.16.0" axum = "0.7.3" lambda_http = "0.9.0" lambda_runtime = "0.9.0" +mockall = "0.12.1" serde = { version = "1.0", features = ["derive"] } serde_dynamo = { version = "4.2.13", features = ["aws-sdk-dynamodb+1"] } serde_json = "1.0.111" From d5fffe3f15b9392893ac42d5b0936f0894c260c4 Mon Sep 17 00:00:00 2001 From: Nicholas Chitty Date: Sat, 29 Jun 2024 16:22:47 -0400 Subject: [PATCH 2/5] Create dynamodb client trait and impl --- lambda/src/aws_client/mod.rs | 84 ++++++++++++++++++++++++++++++++++++ lambda/src/lib.rs | 1 + 2 files changed, 85 insertions(+) create mode 100644 lambda/src/aws_client/mod.rs diff --git a/lambda/src/aws_client/mod.rs b/lambda/src/aws_client/mod.rs new file mode 100644 index 0000000..ecd422a --- /dev/null +++ b/lambda/src/aws_client/mod.rs @@ -0,0 +1,84 @@ +use std::collections::HashMap; + +use aws_config::SdkConfig; +use aws_sdk_dynamodb::error::SdkError; +use aws_sdk_dynamodb::operation::delete_item::{DeleteItemError, DeleteItemOutput}; +use aws_sdk_dynamodb::operation::get_item::{GetItemError, GetItemOutput}; +use aws_sdk_dynamodb::operation::put_item::{PutItemError, PutItemOutput}; +use aws_sdk_dynamodb::types::AttributeValue; +use aws_sdk_dynamodb::Client; +use axum::async_trait; + +#[cfg_attr(test, mockall::automock)] +#[async_trait] +pub trait DynamoDbClient: Send + Sync { + async fn get_item( + &self, + table_name: &str, + key: HashMap, + ) -> Result; + async fn put_item( + &self, + table_name: &str, + item: HashMap, + ) -> Result; + + async fn delete_item( + &self, + table_name: &str, + key: HashMap, + ) -> Result; +} + +#[derive(Clone)] +pub struct DynamoDbClientImpl(Client); + +impl DynamoDbClientImpl { + pub fn new(sdk_config: &SdkConfig) -> Self { Self(Client::new(sdk_config)) } +} + +#[async_trait] +impl DynamoDbClient for DynamoDbClientImpl { + async fn get_item( + &self, + table_name: &str, + key: HashMap, + ) -> Result { + self.0 + .get_item() + .table_name(table_name) + .set_key(Some(key)) + .send() + .await + .map_err(SdkError::into_service_error) + } + + async fn put_item( + &self, + table_name: &str, + item: HashMap, + ) -> Result { + self.0 + .put_item() + .table_name(table_name) + .set_item(Some(item)) + .send() + .await + .map_err(SdkError::into_service_error) + } + + async fn delete_item( + &self, + table_name: &str, + key: HashMap, + ) -> Result { + self.0 + .delete_item() + .table_name(table_name) + .set_key(Some(key)) + .condition_expression("attribute_exists(id)") + .send() + .await + .map_err(SdkError::into_service_error) + } +} diff --git a/lambda/src/lib.rs b/lambda/src/lib.rs index 7418692..ed19e54 100644 --- a/lambda/src/lib.rs +++ b/lambda/src/lib.rs @@ -3,6 +3,7 @@ use std::future::Future; use axum::http::StatusCode; use uuid::Uuid; +pub mod aws_client; pub mod recipe; pub mod services; From c825443363b01fd7223191ed43f01d45b80b9846 Mon Sep 17 00:00:00 2001 From: Nicholas Chitty Date: Sat, 29 Jun 2024 16:23:09 -0400 Subject: [PATCH 3/5] Update and test --- lambda/src/recipe/repository.rs | 202 +++++++++++++++++++++++++++++--- 1 file changed, 184 insertions(+), 18 deletions(-) diff --git a/lambda/src/recipe/repository.rs b/lambda/src/recipe/repository.rs index 933f7fb..bf62643 100644 --- a/lambda/src/recipe/repository.rs +++ b/lambda/src/recipe/repository.rs @@ -1,18 +1,21 @@ +use std::collections::HashMap; +use std::sync::Arc; + use aws_config::SdkConfig; use aws_sdk_dynamodb::operation::delete_item::DeleteItemError; use aws_sdk_dynamodb::operation::get_item::GetItemError; use aws_sdk_dynamodb::types::AttributeValue; -use aws_sdk_dynamodb::Client; use axum::http::StatusCode; use serde_dynamo::aws_sdk_dynamodb_1::{from_item, to_item}; use uuid::Uuid; use super::Recipe; +use crate::aws_client::{DynamoDbClient, DynamoDbClientImpl}; use crate::Repository; #[derive(Clone)] pub struct DynamoDbRecipe { - client: Client, + client: Arc, table_name: String, } @@ -20,7 +23,15 @@ impl DynamoDbRecipe { #[must_use] pub fn new(sdk_config: &SdkConfig, table_name: &str) -> Self { Self { - client: Client::new(sdk_config), + client: Arc::new(DynamoDbClientImpl::new(sdk_config)), + table_name: table_name.to_owned(), + } + } + + #[allow(dead_code)] + fn mock(client: Arc, table_name: &str) -> Self { + Self { + client, table_name: table_name.to_owned(), } } @@ -30,12 +41,9 @@ impl Repository for DynamoDbRecipe { async fn find_by_id(&self, id: Uuid) -> Result { let get_item_result = self .client - .get_item() - .table_name(&self.table_name) - .key("id", AttributeValue::S(id.as_hyphenated().to_string())) - .send() + .get_item(&self.table_name, DynamoDbRecipe::get_key(id)) .await - .map_err(|sdk_err| match sdk_err.into_service_error() { + .map_err(|err| match err { GetItemError::ResourceNotFoundException(_) => StatusCode::NOT_FOUND, _ => StatusCode::INTERNAL_SERVER_ERROR, })?; @@ -52,10 +60,7 @@ impl Repository for DynamoDbRecipe { async fn save(&self, recipe: &Recipe) -> Result<(), StatusCode> { let item = to_item(recipe).map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; self.client - .put_item() - .table_name(&self.table_name) - .set_item(Some(item)) - .send() + .put_item(&self.table_name, item) .await .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; @@ -64,13 +69,9 @@ impl Repository for DynamoDbRecipe { async fn delete_by_id(&self, id: Uuid) -> Result<(), StatusCode> { self.client - .delete_item() - .table_name(&self.table_name) - .key("id", AttributeValue::S(id.as_hyphenated().to_string())) - .condition_expression("attribute_exists(id)") - .send() + .delete_item(&self.table_name, DynamoDbRecipe::get_key(id)) .await - .map_err(|sdk_err| match sdk_err.into_service_error() { + .map_err(|err| match err { DeleteItemError::ConditionalCheckFailedException(_) | DeleteItemError::ResourceNotFoundException(_) => StatusCode::NOT_FOUND, _ => StatusCode::INTERNAL_SERVER_ERROR, @@ -79,3 +80,168 @@ impl Repository for DynamoDbRecipe { Ok(()) } } + +impl DynamoDbRecipe { + fn get_key(id: Uuid) -> HashMap { + let mut key = HashMap::new(); + key.insert( + "id".to_string(), + AttributeValue::S(id.as_hyphenated().to_string()), + ); + + key + } +} + +#[cfg(test)] +mod test { + + use aws_sdk_dynamodb::operation::delete_item::DeleteItemOutput; + use aws_sdk_dynamodb::operation::get_item::GetItemOutput; + use aws_sdk_dynamodb::operation::put_item::{PutItemError, PutItemOutput}; + use aws_sdk_dynamodb::types::error::{ConditionalCheckFailedException, ResourceNotFoundException}; + use mockall::predicate::eq; + + use super::*; + use crate::aws_client::MockDynamoDbClient as DynamoDbClient; + + #[tokio::test] + async fn test_get_no_error() { + let recipe = Recipe { + id: Uuid::nil(), + name: "Name".to_owned(), + }; + let item = to_item(&recipe).unwrap(); + let mut mock = DynamoDbClient::default(); + mock.expect_get_item() + .with(eq("recipes"), eq(DynamoDbRecipe::get_key(Uuid::nil()))) + .return_once(|_, _| Ok(GetItemOutput::builder().set_item(Some(item)).build())); + + let repo = DynamoDbRecipe::mock(Arc::new(mock), "recipes"); + + let result = repo.find_by_id(Uuid::nil()).await; + assert!(result.is_ok()); + assert_eq!(result.unwrap(), recipe); + } + + #[tokio::test] + async fn test_get_not_found() { + let mut mock = DynamoDbClient::default(); + mock.expect_get_item() + .with(eq("recipes"), eq(DynamoDbRecipe::get_key(Uuid::nil()))) + .return_once(|_, _| { + Err(GetItemError::ResourceNotFoundException( + ResourceNotFoundException::builder().build(), + )) + }); + + let repo = DynamoDbRecipe::mock(Arc::new(mock), "recipes"); + + let result = repo.find_by_id(Uuid::nil()).await; + assert!(result.is_err()); + let test = match result { + Err(StatusCode::NOT_FOUND) => true, + _ => false, + }; + assert!(test); + } + + #[tokio::test] + async fn test_save() { + let recipe = Recipe { + id: Uuid::nil(), + name: "Name".to_owned(), + }; + let item = to_item(&recipe).unwrap(); + let mut mock = DynamoDbClient::default(); + mock.expect_put_item() + .with(eq("recipes"), eq(item.clone())) + .return_once(move |_, _| { + Ok(PutItemOutput::builder() + .set_attributes(Some(item.clone())) + .build()) + }); + + let repo = DynamoDbRecipe::mock(Arc::new(mock), "recipes"); + + assert!(repo.save(&recipe).await.is_ok()) + } + + #[tokio::test] + async fn test_save_error() { + let recipe = Recipe { + id: Uuid::nil(), + name: "Name".to_owned(), + }; + let item = to_item(&recipe).unwrap(); + let mut mock = DynamoDbClient::default(); + mock.expect_put_item() + .with(eq("recipes"), eq(item.clone())) + .return_once(move |_, _| { + Err(PutItemError::ResourceNotFoundException( + ResourceNotFoundException::builder().build(), + )) + }); + + let repo = DynamoDbRecipe::mock(Arc::new(mock), "recipes"); + + let result = repo.save(&recipe).await; + assert!(result.is_err_and(|err| match err { + StatusCode::INTERNAL_SERVER_ERROR => true, + _ => false, + })) + } + + #[tokio::test] + async fn test_delete() { + let mut mock = DynamoDbClient::default(); + mock.expect_delete_item() + .with(eq("recipes"), eq(DynamoDbRecipe::get_key(Uuid::nil()))) + .return_once(move |_, _| Ok(DeleteItemOutput::builder().build())); + + let repo = DynamoDbRecipe::mock(Arc::new(mock), "recipes"); + + let result = repo.delete_by_id(Uuid::nil()).await; + assert!(result.is_ok()) + } + + #[tokio::test] + async fn test_delete_error_resource_not_found() { + let mut mock = DynamoDbClient::default(); + mock.expect_delete_item() + .with(eq("recipes"), eq(DynamoDbRecipe::get_key(Uuid::nil()))) + .return_once(move |_, _| { + Err(DeleteItemError::ResourceNotFoundException( + ResourceNotFoundException::builder().build(), + )) + }); + + let repo = DynamoDbRecipe::mock(Arc::new(mock), "recipes"); + + let result = repo.delete_by_id(Uuid::nil()).await; + assert!(result.is_err_and(|err| match err { + StatusCode::NOT_FOUND => true, + _ => false + })) + } + + #[tokio::test] + async fn test_delete_error_condition_unmet() { + let mut mock = DynamoDbClient::default(); + mock.expect_delete_item() + .with(eq("recipes"), eq(DynamoDbRecipe::get_key(Uuid::nil()))) + .return_once(move |_, _| { + Err(DeleteItemError::ConditionalCheckFailedException( + ConditionalCheckFailedException::builder().build(), + )) + }); + + let repo = DynamoDbRecipe::mock(Arc::new(mock), "recipes"); + + let result = repo.delete_by_id(Uuid::nil()).await; + assert!(result.is_err_and(|err| match err { + StatusCode::NOT_FOUND => true, + _ => false + })) + } +} From 182988fa69e829efe5c4881eed54f83224011aef Mon Sep 17 00:00:00 2001 From: Nicholas Chitty Date: Sat, 29 Jun 2024 16:34:04 -0400 Subject: [PATCH 4/5] Cargo fmt --- lambda/src/recipe/repository.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lambda/src/recipe/repository.rs b/lambda/src/recipe/repository.rs index bf62643..8f64e99 100644 --- a/lambda/src/recipe/repository.rs +++ b/lambda/src/recipe/repository.rs @@ -99,7 +99,10 @@ mod test { use aws_sdk_dynamodb::operation::delete_item::DeleteItemOutput; use aws_sdk_dynamodb::operation::get_item::GetItemOutput; use aws_sdk_dynamodb::operation::put_item::{PutItemError, PutItemOutput}; - use aws_sdk_dynamodb::types::error::{ConditionalCheckFailedException, ResourceNotFoundException}; + use aws_sdk_dynamodb::types::error::{ + ConditionalCheckFailedException, + ResourceNotFoundException, + }; use mockall::predicate::eq; use super::*; @@ -221,7 +224,7 @@ mod test { let result = repo.delete_by_id(Uuid::nil()).await; assert!(result.is_err_and(|err| match err { StatusCode::NOT_FOUND => true, - _ => false + _ => false, })) } @@ -241,7 +244,7 @@ mod test { let result = repo.delete_by_id(Uuid::nil()).await; assert!(result.is_err_and(|err| match err { StatusCode::NOT_FOUND => true, - _ => false + _ => false, })) } } From 3426bd5fb2fbba5d4d40504fd4cdc88b8b444fed Mon Sep 17 00:00:00 2001 From: Nicholas Chitty Date: Sat, 29 Jun 2024 16:35:55 -0400 Subject: [PATCH 5/5] Code review comment --- lambda/src/aws_client/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lambda/src/aws_client/mod.rs b/lambda/src/aws_client/mod.rs index ecd422a..8d7e0e1 100644 --- a/lambda/src/aws_client/mod.rs +++ b/lambda/src/aws_client/mod.rs @@ -34,6 +34,7 @@ pub trait DynamoDbClient: Send + Sync { pub struct DynamoDbClientImpl(Client); impl DynamoDbClientImpl { + #[must_use] pub fn new(sdk_config: &SdkConfig) -> Self { Self(Client::new(sdk_config)) } }