From 291d13b962840e5154e5de76430e0a17dbc944cd Mon Sep 17 00:00:00 2001 From: Guillaume Deconinck Date: Thu, 26 Feb 2026 22:59:49 +0900 Subject: [PATCH] fix: some issues --- .../tests/requirements/requirements.spec.ts | 3 + crates/api/src/error.rs | 2 +- crates/api/src/lib.rs | 84 ++++++------------ .../src/user/get_multiple_users_freebusy.rs | 86 ++++++++++++++++--- crates/api/src/user/get_user_freebusy.rs | 24 +++++- 5 files changed, 127 insertions(+), 72 deletions(-) diff --git a/clients/javascript/tests/requirements/requirements.spec.ts b/clients/javascript/tests/requirements/requirements.spec.ts index c03e2f63..5c49d1b0 100644 --- a/clients/javascript/tests/requirements/requirements.spec.ts +++ b/clients/javascript/tests/requirements/requirements.spec.ts @@ -352,6 +352,7 @@ describe('Requirements', () => { calendarId: user1Calendar1.id, duration: 1000 * 60 * 60, startTime: new Date(0), + status: 'confirmed', busy: true, }) user1Calendar1Event1 = resEvent1?.event @@ -624,6 +625,7 @@ describe('Requirements', () => { calendarId: user1Calendar1.id, duration: 1000 * 60 * 60, startTime: new Date(0), + status: 'confirmed', busy: true, }) expect(resEvent1?.event.calendarId).toBe(user1Calendar1.id) @@ -633,6 +635,7 @@ describe('Requirements', () => { calendarId: user2Calendar1.id, duration: 1000 * 60 * 60, startTime: new Date(1000 * 60 * 61), + status: 'confirmed', busy: true, }) expect(resEvent2?.event.calendarId).toBe(user2Calendar1.id) diff --git a/crates/api/src/error.rs b/crates/api/src/error.rs index 31d6fea9..2917fc57 100644 --- a/crates/api/src/error.rs +++ b/crates/api/src/error.rs @@ -38,7 +38,7 @@ impl NitteiError { fn error_response(&self) -> impl IntoResponse { ( self.status_code(), - [(header::CONTENT_TYPE, "text/html; charset=utf-8")], + [(header::CONTENT_TYPE, "text/plain; charset=utf-8")], self.to_string(), ) } diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index 6e5614d9..be103bb8 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -12,8 +12,8 @@ mod user; use std::sync::Arc; +use anyhow::anyhow; use axum::{Extension, Router, http::header}; -use futures::lock::Mutex; use http_logger::metadata_middleware; use job_schedulers::{start_reminder_generation_job, start_send_reminders_job}; use nittei_domain::{ @@ -27,7 +27,10 @@ use nittei_domain::{ use nittei_infra::NitteiContext; use tokio::{ net::TcpListener, - sync::oneshot::{self, Sender}, + sync::{ + Mutex, + oneshot::{self, Sender}, + }, }; use tower::ServiceBuilder; use tower_http::{ @@ -210,11 +213,10 @@ impl Application { info!("[server] Server stopped"); }) .await - .unwrap_or_else(|e| { + .map_err(|e| { error!("[server] Server error: {:?}", e); - // Exit the process with an error code - std::process::exit(1); - }); + anyhow!("Server error: {e:?}") + })?; info!("[server] Server closed"); @@ -293,71 +295,37 @@ impl Application { self.context.repos.accounts.insert(&account).await?; - let account_google_client_id_env = "NITTEI__ACCOUNT__GOOGLE__CLIENT_ID"; - let account_google_client_secret_env = "NITTEI__ACCOUNT__GOOGLE__CLIENT_SECRET"; - let account_google_redirect_uri_env = "NITTEI__ACCOUNT__GOOGLE__REDIRECT_URI"; let google_config = nittei_utils::config::APP_CONFIG .account .as_ref() .and_then(|a| a.google.as_ref()); - if let Some(google_client_id) = google_config.map(|g| g.client_id.clone()) { - let google_client_secret = google_config - .map(|g| g.client_secret.clone()) - .unwrap_or_else(|| { - panic!( - "{account_google_client_secret_env} should be specified also when {account_google_client_id_env} is specified." - ) - }); - let google_redirect_uri = google_config - .map(|g| g.redirect_uri.clone()) - .unwrap_or_else(|| { - panic!( - "{account_google_redirect_uri_env} should be specified also when {account_google_client_id_env} is specified." - ) - }); + if let Some(google_config) = google_config { self.context .repos .account_integrations .insert(&AccountIntegration { account_id: account.id.clone(), - client_id: google_client_id, - client_secret: google_client_secret, - redirect_uri: google_redirect_uri, + client_id: google_config.client_id.clone(), + client_secret: google_config.client_secret.clone(), + redirect_uri: google_config.redirect_uri.clone(), provider: IntegrationProvider::Google, }) .await?; } - let account_outlook_client_id_env = "NITTEI__ACCOUNT__OUTLOOK__CLIENT_ID"; - let account_outlook_client_secret_env = "NITTEI__ACCOUNT__OUTLOOK__CLIENT_SECRET"; - let account_outlook_redirect_uri_env = "NITTEI__ACCOUNT__OUTLOOK__REDIRECT_URI"; let outlook_config = nittei_utils::config::APP_CONFIG .account .as_ref() .and_then(|a| a.outlook.as_ref()); - if let Some(outlook_client_id) = outlook_config.map(|o| o.client_id.clone()) { - let outlook_client_secret = outlook_config - .map(|o| o.client_secret.clone()) - .unwrap_or_else(|| { - panic!( - "{account_outlook_client_secret_env} should be specified also when {account_outlook_client_id_env} is specified." - ) - }); - let outlook_redirect_uri = outlook_config - .map(|o| o.redirect_uri.clone()) - .unwrap_or_else(|| { - panic!( - "{account_outlook_redirect_uri_env} should be specified also when {account_outlook_client_id_env} is specified." - ) - }); + if let Some(outlook_config) = outlook_config { self.context .repos .account_integrations .insert(&AccountIntegration { account_id: account.id.clone(), - client_id: outlook_client_id, - client_secret: outlook_client_secret, - redirect_uri: outlook_redirect_uri, + client_id: outlook_config.client_id.clone(), + client_secret: outlook_config.client_secret.clone(), + redirect_uri: outlook_config.redirect_uri.clone(), provider: IntegrationProvider::Outlook, }) .await?; @@ -401,12 +369,10 @@ impl Application { if cfg!(debug_assertions) { // In debug mode, stop the server immediately info!("[shutdown_handler] Stopping server..."); - if let Some(server_handle) = shared_state.lock().await.shutdown_tx.take() { - server_handle.send(()).unwrap_or_else(|_| { - error!("[shutdown_handler] Failed to send shutdown signal"); - // Exit the process with an error code - std::process::exit(1); - }); + if let Some(server_handle) = shared_state.lock().await.shutdown_tx.take() + && server_handle.send(()).is_err() + { + error!("[shutdown_handler] Failed to send shutdown signal"); } info!("[shutdown_handler] api crate - shutdown complete"); } else { @@ -422,12 +388,10 @@ impl Application { info!("[shutdown_handler] Stopping server..."); // Shutdown the server - if let Some(server_handle) = shared_state.lock().await.shutdown_tx.take() { - server_handle.send(()).unwrap_or_else(|_| { - error!("[shutdown_handler] Failed to send shutdown signal"); - // Exit the process with an error code - std::process::exit(1); - }); + if let Some(server_handle) = shared_state.lock().await.shutdown_tx.take() + && server_handle.send(()).is_err() + { + error!("[shutdown_handler] Failed to send shutdown signal"); } info!("[shutdown_handler] api crate - shutdown complete"); diff --git a/crates/api/src/user/get_multiple_users_freebusy.rs b/crates/api/src/user/get_multiple_users_freebusy.rs index 8840fc86..5496ae07 100644 --- a/crates/api/src/user/get_multiple_users_freebusy.rs +++ b/crates/api/src/user/get_multiple_users_freebusy.rs @@ -47,9 +47,10 @@ pub async fn get_multiple_freebusy_controller( Extension(ctx): Extension, body: Json, ) -> Result, NitteiError> { - let _account = protect_public_account_route(&headers, &ctx).await?; + let account = protect_public_account_route(&headers, &ctx).await?; let usecase = GetMultipleFreeBusyUseCase { + account_id: account.id, user_ids: body.user_ids.clone(), start_time: body.start_time, end_time: body.end_time, @@ -63,6 +64,7 @@ pub async fn get_multiple_freebusy_controller( #[derive(Debug)] pub struct GetMultipleFreeBusyUseCase { + pub account_id: ID, pub user_ids: Vec, pub start_time: DateTime, pub end_time: DateTime, @@ -75,6 +77,7 @@ pub struct GetMultipleFreeBusyResponse(pub HashMap>); pub enum UseCaseError { InternalError, InvalidTimespan, + UserNotFound(ID), } impl From for NitteiError { @@ -84,6 +87,9 @@ impl From for NitteiError { UseCaseError::InvalidTimespan => { Self::BadClientData("The provided start_ts and end_ts are invalid".into()) } + UseCaseError::UserNotFound(user_id) => { + Self::NotFound(format!("A user with id: {user_id}, was not found.")) + } } } } @@ -97,6 +103,8 @@ impl UseCase for GetMultipleFreeBusyUseCase { const NAME: &'static str = "GetMultipleFreebusy"; async fn execute(&mut self, ctx: &NitteiContext) -> Result { + self.ensure_users_belong_to_account(ctx).await?; + let timespan = TimeSpan::new(self.start_time, self.end_time); if timespan.greater_than(APP_CONFIG.event_instances_query_duration_limit) { return Err(UseCaseError::InvalidTimespan); @@ -121,6 +129,21 @@ impl UseCase for GetMultipleFreeBusyUseCase { } impl GetMultipleFreeBusyUseCase { + async fn ensure_users_belong_to_account(&self, ctx: &NitteiContext) -> Result<(), UseCaseError> { + for user_id in &self.user_ids { + let user = ctx + .repos + .users + .find_by_account_id(user_id, &self.account_id) + .await + .map_err(|_| UseCaseError::InternalError)?; + if user.is_none() { + return Err(UseCaseError::UserNotFound(user_id.clone())); + } + } + Ok(()) + } + async fn get_calendars_from_user_ids( &self, ctx: &NitteiContext, @@ -160,19 +183,33 @@ impl GetMultipleFreeBusyUseCase { // End result let mut events_per_user: HashMap> = HashMap::new(); - // Fetch all events for all calendars + // Group calendars by user once so each user is queried with the same semantics + // as the single-user freebusy endpoint. + let mut calendar_ids_by_user: HashMap> = HashMap::new(); + for calendar in &calendars { + calendar_ids_by_user + .entry(calendar.user_id.clone()) + .or_default() + .push(calendar.id.clone()); + } + + // Fetch all events for all users (using calendar groups) // This is not executed yet (lazy) - let all_events_futures = calendars.clone().into_iter().map(|calendar| { + let all_events_futures = calendar_ids_by_user.into_iter().map(|(user_id, calendar_ids)| { let timespan = timespan.clone(); async move { let events = ctx .repos .events - .find_by_calendar(&calendar.id, Some(timespan)) + .find_busy_events_and_recurring_events_for_calendars( + &calendar_ids, + timespan, + false, + ) .await - .unwrap_or_default(); // TODO: Handle error - Ok((calendar.user_id.clone(), events)) + .map_err(|_| UseCaseError::InternalError)?; + Ok((user_id, events)) as Result<(ID, Vec), UseCaseError> } .boxed() @@ -198,7 +235,10 @@ impl GetMultipleFreeBusyUseCase { error!("Got an error when expanding events {:?}", e); UseCaseError::InternalError })?; - events_per_user.insert(user_id, expanded_events); + events_per_user + .entry(user_id) + .or_default() + .extend(expanded_events); } Err(e) => return Err(e), } @@ -230,12 +270,15 @@ mod test { ctx.repos.users.insert(&user).await.unwrap(); let calendar = Calendar::new(&user.id(), &user.account_id, None, None); ctx.repos.calendars.insert(&calendar).await.unwrap(); + let second_calendar = Calendar::new(&user.id(), &user.account_id, None, None); + ctx.repos.calendars.insert(&second_calendar).await.unwrap(); let one_hour = 1000 * 60 * 60; let mut e1 = CalendarEvent { calendar_id: calendar.id.clone(), user_id: user.id.clone(), account_id: user.account_id.clone(), busy: true, + status: nittei_domain::CalendarEventStatus::Confirmed, duration: one_hour, end_time: DateTime::::MAX_UTC, ..Default::default() @@ -256,6 +299,7 @@ mod test { user_id: user.id.clone(), account_id: user.account_id.clone(), busy: true, + status: nittei_domain::CalendarEventStatus::Confirmed, duration: one_hour, end_time: DateTime::::MAX_UTC, start_time: DateTime::from_timestamp_millis(one_hour * 4).unwrap(), @@ -277,6 +321,7 @@ mod test { user_id: user.id.clone(), account_id: user.account_id.clone(), busy: true, + status: nittei_domain::CalendarEventStatus::Confirmed, duration: one_hour, end_time: DateTime::from_timestamp_millis(one_hour).unwrap(), ..Default::default() @@ -292,12 +337,25 @@ mod test { panic!("{e:?}"); } }; + let e4 = CalendarEvent { + calendar_id: second_calendar.id.clone(), + user_id: user.id.clone(), + account_id: user.account_id.clone(), + busy: true, + status: nittei_domain::CalendarEventStatus::Confirmed, + start_time: DateTime::from_timestamp_millis(90000000).unwrap(), + end_time: DateTime::from_timestamp_millis(93600000).unwrap(), + duration: one_hour, + ..Default::default() + }; ctx.repos.events.insert(&e1).await.unwrap(); ctx.repos.events.insert(&e2).await.unwrap(); ctx.repos.events.insert(&e3).await.unwrap(); + ctx.repos.events.insert(&e4).await.unwrap(); let mut usecase = GetMultipleFreeBusyUseCase { + account_id: account.id.clone(), user_ids: vec![user.id().clone()], start_time: DateTime::from_timestamp_millis(86400000).unwrap(), end_time: DateTime::from_timestamp_millis(172800000).unwrap(), @@ -310,7 +368,7 @@ mod test { assert_eq!(map_instances.len(), 1); assert!(map_instances.contains_key(&user.id())); let instances = map_instances.get(&user.id()).unwrap(); - assert_eq!(instances.len(), 4); + assert_eq!(instances.len(), 5); assert_eq!( instances[0], EventInstance { @@ -321,6 +379,14 @@ mod test { ); assert_eq!( instances[1], + EventInstance { + busy: true, + start_time: DateTime::from_timestamp_millis(90000000).unwrap(), + end_time: DateTime::from_timestamp_millis(93600000).unwrap(), + } + ); + assert_eq!( + instances[2], EventInstance { busy: true, start_time: DateTime::from_timestamp_millis(100800000).unwrap(), @@ -328,7 +394,7 @@ mod test { } ); assert_eq!( - instances[2], + instances[3], EventInstance { busy: true, start_time: DateTime::from_timestamp_millis(172800000).unwrap(), @@ -336,7 +402,7 @@ mod test { } ); assert_eq!( - instances[3], + instances[4], EventInstance { busy: true, start_time: DateTime::from_timestamp_millis(172800000).unwrap(), diff --git a/crates/api/src/user/get_user_freebusy.rs b/crates/api/src/user/get_user_freebusy.rs index f6e8a661..89dca4ba 100644 --- a/crates/api/src/user/get_user_freebusy.rs +++ b/crates/api/src/user/get_user_freebusy.rs @@ -61,9 +61,10 @@ pub async fn get_freebusy_controller( mut params: Path, Extension(ctx): Extension, ) -> Result, NitteiError> { - let _account = protect_public_account_route(&headers, &ctx).await?; + let account = protect_public_account_route(&headers, &ctx).await?; let usecase = GetFreeBusyUseCase { + account_id: account.id, user_id: std::mem::take(&mut params.user_id), calendar_ids: query_params.calendar_ids.take(), start_time: query_params.start_time, @@ -84,6 +85,7 @@ pub async fn get_freebusy_controller( #[derive(Debug)] pub struct GetFreeBusyUseCase { + pub account_id: ID, pub user_id: ID, pub calendar_ids: Option>, pub start_time: DateTime, @@ -101,6 +103,7 @@ pub struct GetFreeBusyResponse { pub enum UseCaseError { InternalError, InvalidTimespan, + UserNotFound(ID), } impl From for NitteiError { @@ -110,6 +113,9 @@ impl From for NitteiError { UseCaseError::InvalidTimespan => { Self::BadClientData("The provided start_ts and end_ts are invalid".into()) } + UseCaseError::UserNotFound(user_id) => { + Self::NotFound(format!("A user with id: {user_id}, was not found.")) + } } } } @@ -123,6 +129,8 @@ impl UseCase for GetFreeBusyUseCase { const NAME: &'static str = "GetUserFreebusy"; async fn execute(&mut self, ctx: &NitteiContext) -> Result { + self.ensure_user_belongs_to_account(ctx).await?; + let timespan = TimeSpan::new(self.start_time, self.end_time); if timespan.greater_than(APP_CONFIG.event_instances_query_duration_limit) { return Err(UseCaseError::InvalidTimespan); @@ -146,6 +154,19 @@ impl UseCase for GetFreeBusyUseCase { } impl GetFreeBusyUseCase { + async fn ensure_user_belongs_to_account( + &self, + ctx: &NitteiContext, + ) -> Result<(), UseCaseError> { + ctx.repos + .users + .find_by_account_id(&self.user_id, &self.account_id) + .await + .map_err(|_| UseCaseError::InternalError)? + .ok_or_else(|| UseCaseError::UserNotFound(self.user_id.clone())) + .map(|_| ()) + } + async fn get_event_instances_from_calendars( &self, timespan: TimeSpan, @@ -317,6 +338,7 @@ mod test { .unwrap(); let mut freebusy_params: GetFreeBusyUseCase = GetFreeBusyUseCase { + account_id: account.id.clone(), user_id: user.id().clone(), calendar_ids: Some(vec![calendar.id.clone()]), start_time: DateTime::parse_from_rfc3339("2025-01-09T00:00:00Z")