From bc8a534353289d0dede02a0e93ba7be8082dd8fe Mon Sep 17 00:00:00 2001 From: Simon Goller Date: Thu, 9 May 2024 08:11:38 +0200 Subject: [PATCH] Add id check for booking It now checks if sales_person_id and slot_id actually exists. --- app/src/main.rs | 5 +- rest/src/booking.rs | 26 +++++---- service/src/lib.rs | 2 + service/src/sales_person.rs | 1 + service/src/slot.rs | 1 + service_impl/src/booking.rs | 71 ++++++++++++++++++++--- service_impl/src/sales_person.rs | 8 +++ service_impl/src/slot.rs | 5 ++ service_impl/src/test/booking.rs | 83 +++++++++++++++++++++++++-- service_impl/src/test/sales_person.rs | 23 ++++++++ 10 files changed, 201 insertions(+), 24 deletions(-) diff --git a/app/src/main.rs b/app/src/main.rs index 0213c0a..602fcf7 100644 --- a/app/src/main.rs +++ b/app/src/main.rs @@ -23,6 +23,8 @@ type BookingService = service_impl::booking::BookingServiceImpl< PermissionService, ClockService, UuidService, + SalesPersonService, + SlotService, >; #[derive(Clone)] @@ -31,7 +33,6 @@ pub struct RestStateImpl { slot_service: Arc, sales_person_service: Arc, booking_service: Arc, - } impl rest::RestStateDef for RestStateImpl { type PermissionService = PermissionService; @@ -90,6 +91,8 @@ impl RestStateImpl { permission_service.clone(), clock_service, uuid_service, + sales_person_service.clone(), + slot_service.clone(), )); Self { permission_service, diff --git a/rest/src/booking.rs b/rest/src/booking.rs index 3283ea4..1838ad7 100644 --- a/rest/src/booking.rs +++ b/rest/src/booking.rs @@ -10,7 +10,7 @@ use time::PrimitiveDateTime; use uuid::Uuid; use crate::{error_handler, RestStateDef}; -use service::booking::{BookingService, Booking}; +use service::booking::{Booking, BookingService}; #[derive(Serialize, Deserialize, Clone, Debug)] pub struct BookingTO { @@ -65,9 +65,7 @@ pub fn generate_route() -> Router { .route("/:id", delete(delete_booking::)) } -pub async fn get_all_bookings( - rest_state: State, -) -> Response { +pub async fn get_all_bookings(rest_state: State) -> Response { error_handler( (async { let bookings: Arc<[BookingTO]> = rest_state @@ -95,7 +93,9 @@ pub async fn get_booking( let booking = rest_state.booking_service().get(booking_id, ()).await?; Ok(Response::builder() .status(200) - .body(Body::new(serde_json::to_string(&BookingTO::from(&booking)).unwrap())) + .body(Body::new( + serde_json::to_string(&BookingTO::from(&booking)).unwrap(), + )) .unwrap()) }) .await, @@ -108,10 +108,15 @@ pub async fn create_booking( ) -> Response { error_handler( (async { - let booking = rest_state.booking_service().create(&Booking::from(&booking), ()).await?; + let booking = rest_state + .booking_service() + .create(&Booking::from(&booking), ()) + .await?; Ok(Response::builder() .status(200) - .body(Body::new(serde_json::to_string(&BookingTO::from(&booking)).unwrap())) + .body(Body::new( + serde_json::to_string(&BookingTO::from(&booking)).unwrap(), + )) .unwrap()) }) .await, @@ -125,11 +130,8 @@ pub async fn delete_booking( error_handler( (async { rest_state.booking_service().delete(booking_id, ()).await?; - Ok(Response::builder() - .status(200) - .body(Body::empty()) - .unwrap()) + Ok(Response::builder().status(200).body(Body::empty()).unwrap()) }) .await, ) -} \ No newline at end of file +} diff --git a/service/src/lib.rs b/service/src/lib.rs index e7b6b2c..d891c23 100644 --- a/service/src/lib.rs +++ b/service/src/lib.rs @@ -22,6 +22,8 @@ pub use permission::User; pub enum ValidationFailureItem { ModificationNotAllowed(Arc), InvalidValue(Arc), + IdDoesNotExist(Arc, Uuid), + Duplicate, } #[derive(Debug, Error)] diff --git a/service/src/sales_person.rs b/service/src/sales_person.rs index daedaea..b3ac790 100644 --- a/service/src/sales_person.rs +++ b/service/src/sales_person.rs @@ -44,6 +44,7 @@ pub trait SalesPersonService { async fn get_all(&self, context: Self::Context) -> Result, ServiceError>; async fn get(&self, id: Uuid, context: Self::Context) -> Result; + async fn exists(&self, id: Uuid, context: Self::Context) -> Result; async fn create( &self, item: &SalesPerson, diff --git a/service/src/slot.rs b/service/src/slot.rs index 6fd7e28..48b7f36 100644 --- a/service/src/slot.rs +++ b/service/src/slot.rs @@ -89,6 +89,7 @@ pub trait SlotService { async fn get_slots(&self, context: Self::Context) -> Result, ServiceError>; async fn get_slot(&self, id: &Uuid, context: Self::Context) -> Result; + async fn exists(&self, id: Uuid, context: Self::Context) -> Result; async fn create_slot(&self, slot: &Slot, context: Self::Context) -> Result; async fn delete_slot(&self, id: &Uuid, context: Self::Context) -> Result<(), ServiceError>; async fn update_slot(&self, slot: &Slot, context: Self::Context) -> Result<(), ServiceError>; diff --git a/service_impl/src/booking.rs b/service_impl/src/booking.rs index 53fa4c2..ceaf112 100644 --- a/service_impl/src/booking.rs +++ b/service_impl/src/booking.rs @@ -8,49 +8,84 @@ use uuid::Uuid; const BOOKING_SERVICE_PROCESS: &str = "booking-service"; -pub struct BookingServiceImpl -where +pub struct BookingServiceImpl< + BookingDao, + PermissionService, + ClockService, + UuidService, + SalesPersonService, + SlotService, +> where BookingDao: dao::booking::BookingDao + Send + Sync, PermissionService: service::permission::PermissionService + Send + Sync, UuidService: service::uuid_service::UuidService + Send + Sync, ClockService: service::clock::ClockService + Send + Sync, + SalesPersonService: service::sales_person::SalesPersonService + Send + Sync, + SlotService: service::slot::SlotService + Send + Sync, { pub booking_dao: Arc, pub permission_service: Arc, pub clock_service: Arc, pub uuid_service: Arc, + pub sales_person_service: Arc, + pub slot_service: Arc, } -impl - BookingServiceImpl +impl + BookingServiceImpl< + BookingDao, + PermissionService, + ClockService, + UuidService, + SalesPersonService, + SlotService, + > where BookingDao: dao::booking::BookingDao + Send + Sync, PermissionService: service::permission::PermissionService + Send + Sync, UuidService: service::uuid_service::UuidService + Send + Sync, ClockService: service::clock::ClockService + Send + Sync, + SalesPersonService: service::sales_person::SalesPersonService + Send + Sync, + SlotService: service::slot::SlotService + Send + Sync, { pub fn new( booking_dao: Arc, permission_service: Arc, clock_service: Arc, uuid_service: Arc, + sales_person_service: Arc, + slot_service: Arc, ) -> Self { Self { booking_dao, permission_service, clock_service, uuid_service, + sales_person_service, + slot_service, } } } #[async_trait] -impl BookingService - for BookingServiceImpl +impl + BookingService + for BookingServiceImpl< + BookingDao, + PermissionService, + ClockService, + UuidService, + SalesPersonService, + SlotService, + > where BookingDao: dao::booking::BookingDao + Send + Sync, PermissionService: service::permission::PermissionService + Send + Sync, UuidService: service::uuid_service::UuidService + Send + Sync, ClockService: service::clock::ClockService + Send + Sync, + SalesPersonService: service::sales_person::SalesPersonService + + Send + + Sync, + SlotService: service::slot::SlotService + Send + Sync, { type Context = PermissionService::Context; @@ -85,7 +120,7 @@ where context: Self::Context, ) -> Result { self.permission_service - .check_permission("hr", context) + .check_permission("hr", context.clone()) .await?; if booking.id != Uuid::nil() { @@ -113,6 +148,28 @@ where if booking.calendar_week > 53 { validation.push(ValidationFailureItem::InvalidValue("calendar_week".into())); } + if self + .sales_person_service + .exists(booking.sales_person_id, context.clone()) + .await? + == false + { + validation.push(ValidationFailureItem::IdDoesNotExist( + "sales_person_id".into(), + booking.sales_person_id, + )); + } + if self + .slot_service + .exists(booking.slot_id, context.clone()) + .await? + == false + { + validation.push(ValidationFailureItem::IdDoesNotExist( + "slot_id".into(), + booking.slot_id, + )); + } if !validation.is_empty() { return Err(ServiceError::ValidationError(validation.into())); diff --git a/service_impl/src/sales_person.rs b/service_impl/src/sales_person.rs index c225665..4553556 100644 --- a/service_impl/src/sales_person.rs +++ b/service_impl/src/sales_person.rs @@ -86,6 +86,14 @@ where .ok_or(ServiceError::EntityNotFound(id)) } + async fn exists(&self, id: Uuid, _context: Self::Context) -> Result { + Ok(self + .sales_person_dao + .find_by_id(id) + .await + .map(|x| x.is_some())?) + } + async fn create( &self, sales_person: &SalesPerson, diff --git a/service_impl/src/slot.rs b/service_impl/src/slot.rs index 9e1a0a3..cb95755 100644 --- a/service_impl/src/slot.rs +++ b/service_impl/src/slot.rs @@ -91,6 +91,11 @@ where .ok_or_else(move || ServiceError::EntityNotFound(*id))?; Ok(slot) } + + async fn exists(&self, id: Uuid, _context: Self::Context) -> Result { + Ok(self.slot_dao.get_slot(&id).await.map(|s| s.is_some())?) + } + async fn create_slot(&self, slot: &Slot, context: Self::Context) -> Result { self.permission_service .check_permission("hr", context.clone()) diff --git a/service_impl/src/test/booking.rs b/service_impl/src/test/booking.rs index dfdc6cb..4d7884a 100644 --- a/service_impl/src/test/booking.rs +++ b/service_impl/src/test/booking.rs @@ -2,8 +2,9 @@ use crate::test::error_test::*; use dao::booking::{BookingEntity, MockBookingDao}; use mockall::predicate::eq; use service::{ - booking::Booking, clock::MockClockService, uuid_service::MockUuidService, - MockPermissionService, ValidationFailureItem, + booking::Booking, clock::MockClockService, sales_person::MockSalesPersonService, + slot::MockSlotService, uuid_service::MockUuidService, MockPermissionService, + ValidationFailureItem, }; use time::{Date, Month, PrimitiveDateTime, Time}; use uuid::{uuid, Uuid}; @@ -67,17 +68,27 @@ pub struct BookingServiceDependencies { pub permission_service: MockPermissionService, pub clock_service: MockClockService, pub uuid_service: MockUuidService, + pub sales_person_service: MockSalesPersonService, + pub slot_service: MockSlotService, } impl BookingServiceDependencies { pub fn build_service( self, - ) -> BookingServiceImpl - { + ) -> BookingServiceImpl< + MockBookingDao, + MockPermissionService, + MockClockService, + MockUuidService, + MockSalesPersonService, + MockSlotService, + > { BookingServiceImpl::new( self.booking_dao.into(), self.permission_service.into(), self.clock_service.into(), self.uuid_service.into(), + self.sales_person_service.into(), + self.slot_service.into(), ) } } @@ -113,11 +124,20 @@ pub fn build_dependencies(permission: bool, role: &'static str) -> BookingServic }); let uuid_service = MockUuidService::new(); + let mut sales_person_service = MockSalesPersonService::new(); + sales_person_service + .expect_exists() + .returning(|_, _| Ok(true)); + let mut slot_service = MockSlotService::new(); + slot_service.expect_exists().returning(|_, _| Ok(true)); + BookingServiceDependencies { booking_dao, permission_service, clock_service, uuid_service, + sales_person_service, + slot_service, } } @@ -303,6 +323,61 @@ async fn test_create_with_created_fail() { ); } +#[tokio::test] +async fn test_create_sales_person_does_not_exist() { + let mut deps = build_dependencies(true, "hr"); + deps.sales_person_service.checkpoint(); + deps.sales_person_service + .expect_exists() + .with(eq(default_sales_person_id()), eq(())) + .returning(|_, _| Ok(false)); + let service = deps.build_service(); + let result = service + .create( + &Booking { + id: Uuid::nil(), + version: Uuid::nil(), + created: None, + ..default_booking() + }, + (), + ) + .await; + dbg!(&result); + test_validation_error( + &result, + &ValidationFailureItem::IdDoesNotExist("sales_person_id".into(), default_sales_person_id()), + 1, + ); +} + +#[tokio::test] +async fn test_create_slot_does_not_exist() { + let mut deps = build_dependencies(true, "hr"); + deps.slot_service.checkpoint(); + deps.slot_service + .expect_exists() + .with(eq(default_slot_id()), eq(())) + .returning(|_, _| Ok(false)); + let service = deps.build_service(); + let result = service + .create( + &Booking { + id: Uuid::nil(), + version: Uuid::nil(), + created: None, + ..default_booking() + }, + (), + ) + .await; + test_validation_error( + &result, + &ValidationFailureItem::IdDoesNotExist("slot_id".into(), default_slot_id()), + 1, + ); +} + #[tokio::test] async fn test_delete_no_permission() { let deps = build_dependencies(false, "hr"); diff --git a/service_impl/src/test/sales_person.rs b/service_impl/src/test/sales_person.rs index 1ceb096..583805e 100644 --- a/service_impl/src/test/sales_person.rs +++ b/service_impl/src/test/sales_person.rs @@ -517,3 +517,26 @@ async fn test_delete_not_found() { let result = sales_person_service.delete(default_id(), ()).await; test_not_found(&result, &default_id()); } + +#[tokio::test] +async fn test_exists() { + let mut dependencies = build_dependencies(true, "hr"); + dependencies + .sales_person_dao + .expect_find_by_id() + .with(eq(default_id())) + .returning(|_| Ok(Some(default_sales_person_entity()))); + let sales_person_service = dependencies.build_service(); + let result = sales_person_service.exists(default_id(), ()).await.unwrap(); + assert!(result); + + let mut dependencies = build_dependencies(true, "hr"); + dependencies.sales_person_dao.checkpoint(); + dependencies + .sales_person_dao + .expect_find_by_id() + .with(eq(default_id())) + .returning(|_| Ok(None)); + let result = sales_person_service.exists(default_id(), ()).await.unwrap(); + assert_eq!(false, !result); +}