From 99c6e86b2c9217d4ba0353ca820a8c6d805acf11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luukas=20P=C3=B6rtfors?= Date: Thu, 28 Dec 2023 20:32:03 +0200 Subject: [PATCH] refactor: get rid of leaderboard cache --- src/api/leaderboards.rs | 234 +++++++++++++++++----------------------- src/api/users.rs | 10 +- src/main.rs | 2 - src/tests/mod.rs | 4 +- 4 files changed, 103 insertions(+), 147 deletions(-) diff --git a/src/api/leaderboards.rs b/src/api/leaderboards.rs index 733ab4c..2a3a0d9 100644 --- a/src/api/leaderboards.rs +++ b/src/api/leaderboards.rs @@ -1,17 +1,13 @@ use actix_web::{ error::*, - web::{self, Data, Json, Path}, + web::{self, Json, Path}, HttpResponse, Responder, }; -use dashmap::DashMap; use diesel::result::DatabaseErrorKind; use serde::{Deserialize, Serialize}; use crate::{ - api::auth::SecuredUserIdentity, - database::DatabaseWrapper, - error::TimeError, - models::{PrivateLeaderboard, UserId}, + api::auth::SecuredUserIdentity, database::DatabaseWrapper, error::TimeError, models::UserId, }; #[derive(Deserialize, Serialize)] @@ -29,13 +25,6 @@ pub struct LeaderboardUser { pub user: String, } -pub struct CachedLeaderboard { - pub board: PrivateLeaderboard, - pub valid_until: chrono::DateTime, -} - -pub type LeaderboardCache = DashMap; - #[post("/leaderboards/create")] pub async fn create_leaderboard( creator: UserId, @@ -71,37 +60,17 @@ pub async fn get_leaderboard( user: UserId, path: Path<(String,)>, db: DatabaseWrapper, - cache: Data, ) -> Result { - let name = path.0.clone(); - if let Ok(lid) = db.get_leaderboard_id_by_name(name).await { - if db.is_leaderboard_member(user.id, lid).await? { - if let Some(cached_leaderboard) = cache.get(&lid) { - if cached_leaderboard.value().valid_until > chrono::Utc::now() { - return Ok(web::Json(cached_leaderboard.board.to_owned())); - } else { - drop(cached_leaderboard); - cache.remove(&lid); - } - } - let board = db.get_leaderboard(path.0.clone()).await?; - - cache.insert( - lid, - CachedLeaderboard { - board: board.clone(), - valid_until: chrono::Utc::now() + chrono::Duration::minutes(5), - }, - ); + let lid = db + .get_leaderboard_id_by_name(path.0.clone()) + .await + .map_err(|_| TimeError::LeaderboardNotFound)?; - Ok(web::Json(board)) - } else { - error!("{}", TimeError::Unauthorized); - Err(TimeError::Unauthorized) - } + if db.is_leaderboard_member(user.id, lid).await? { + let board = db.get_leaderboard(path.0.clone()).await?; + Ok(web::Json(board)) } else { - error!("{}", TimeError::LeaderboardNotFound); - Err(TimeError::LeaderboardNotFound) + Err(TimeError::Unauthorized) } } @@ -111,18 +80,16 @@ pub async fn delete_leaderboard( path: Path<(String,)>, db: DatabaseWrapper, ) -> Result { - let name = path.0.clone(); - if let Ok(lid) = db.get_leaderboard_id_by_name(name).await { - if db.is_leaderboard_admin(user.identity.id, lid).await? { - db.delete_leaderboard(path.0.clone()).await?; - Ok(HttpResponse::Ok().finish()) - } else { - error!("{}", TimeError::Unauthorized); - Err(TimeError::Unauthorized) - } + let lid = db + .get_leaderboard_id_by_name(path.0.clone()) + .await + .map_err(|_| TimeError::LeaderboardNotFound)?; + + if db.is_leaderboard_admin(user.identity.id, lid).await? { + db.delete_leaderboard(path.0.clone()).await?; + Ok(HttpResponse::Ok().finish()) } else { - error!("{}", TimeError::LeaderboardNotFound); - Err(TimeError::LeaderboardNotFound) + Err(TimeError::Unauthorized) } } @@ -162,24 +129,24 @@ pub async fn leave_leaderboard( path: Path<(String,)>, db: DatabaseWrapper, ) -> Result { - if let Ok(lid) = db.get_leaderboard_id_by_name(path.0.clone()).await { - if db.is_leaderboard_admin(user.identity.id, lid).await? - && db.get_leaderboard_admin_count(lid).await? == 1 - { - return Err(TimeError::LastAdmin); - } - let left = db - .remove_user_from_leaderboard(lid, user.identity.id) - .await?; + let lid = db + .get_leaderboard_id_by_name(path.0.clone()) + .await + .map_err(|_| TimeError::LeaderboardNotFound)?; - if left { - Ok(HttpResponse::Ok().finish()) - } else { - Err(TimeError::NotMember) - } + if db.is_leaderboard_admin(user.identity.id, lid).await? + && db.get_leaderboard_admin_count(lid).await? == 1 + { + return Err(TimeError::LastAdmin); + } + + if db + .remove_user_from_leaderboard(lid, user.identity.id) + .await? + { + Ok(HttpResponse::Ok().finish()) } else { - error!("{}", TimeError::LeaderboardNotFound); - Err(TimeError::LeaderboardNotFound) + Err(TimeError::NotMember) } } @@ -190,29 +157,28 @@ pub async fn promote_member( db: DatabaseWrapper, promotion: Json, ) -> Result { - if let Ok(lid) = db.get_leaderboard_id_by_name(path.0.clone()).await { - if db.is_leaderboard_admin(user.identity.id, lid).await? { - if let Ok(newadmin) = db.get_user_by_name(promotion.user.clone()).await { - if db - .promote_user_to_leaderboard_admin(lid, newadmin.id) - .await? - { - Ok(HttpResponse::Ok().finish()) - } else { - // FIXME: This is not correct - Err(TimeError::NotMember) - } - } else { - error!("{}", TimeError::UserNotFound); - Err(TimeError::UserNotFound) - } + let lid = db + .get_leaderboard_id_by_name(path.0.clone()) + .await + .map_err(|_| TimeError::LeaderboardNotFound)?; + + if db.is_leaderboard_admin(user.identity.id, lid).await? { + let newadmin = db + .get_user_by_name(promotion.user.clone()) + .await + .map_err(|_| TimeError::UserNotFound)?; + + if db + .promote_user_to_leaderboard_admin(lid, newadmin.id) + .await? + { + Ok(HttpResponse::Ok().finish()) } else { - error!("{}", TimeError::Unauthorized); - Err(TimeError::Unauthorized) + // FIXME: This is not correct + Err(TimeError::NotMember) } } else { - error!("{}", TimeError::LeaderboardNotFound); - Err(TimeError::LeaderboardNotFound) + Err(TimeError::Unauthorized) } } @@ -223,29 +189,28 @@ pub async fn demote_member( db: DatabaseWrapper, demotion: Json, ) -> Result { - if let Ok(lid) = db.get_leaderboard_id_by_name(path.0.clone()).await { - if db.is_leaderboard_admin(user.identity.id, lid).await? { - if let Ok(oldadmin) = db.get_user_by_name(demotion.user.clone()).await { - if db - .demote_user_to_leaderboard_member(lid, oldadmin.id) - .await? - { - Ok(HttpResponse::Ok().finish()) - } else { - // FIXME: This is not correct - Err(TimeError::NotMember) - } - } else { - error!("{}", TimeError::UserNotFound); - Err(TimeError::UserNotFound) - } + let lid = db + .get_leaderboard_id_by_name(path.0.clone()) + .await + .map_err(|_| TimeError::LeaderboardNotFound)?; + + if db.is_leaderboard_admin(user.identity.id, lid).await? { + let oldadmin = db + .get_user_by_name(demotion.user.clone()) + .await + .map_err(|_| TimeError::UserNotFound)?; + + if db + .demote_user_to_leaderboard_member(lid, oldadmin.id) + .await? + { + Ok(HttpResponse::Ok().finish()) } else { - error!("{}", TimeError::Unauthorized); - Err(TimeError::Unauthorized) + // FIXME: This is not correct + Err(TimeError::NotMember) } } else { - error!("{}", TimeError::LeaderboardNotFound); - Err(TimeError::LeaderboardNotFound) + Err(TimeError::Unauthorized) } } @@ -256,25 +221,23 @@ pub async fn kick_member( db: DatabaseWrapper, kick: Json, ) -> Result { - if let Ok(lid) = db.get_leaderboard_id_by_name(path.0.clone()).await { - if db.is_leaderboard_admin(user.identity.id, lid).await? { - if let Ok(kmember) = db.get_user_by_name(kick.user.clone()).await { - if db.remove_user_from_leaderboard(lid, kmember.id).await? { - Ok(HttpResponse::Ok().finish()) - } else { - Err(TimeError::NotMember) - } - } else { - error!("{}", TimeError::UserNotFound); - Err(TimeError::UserNotFound) - } - } else { - error!("{}", TimeError::Unauthorized); - Err(TimeError::Unauthorized) - } + let lid = db + .get_leaderboard_id_by_name(path.0.clone()) + .await + .map_err(|_| TimeError::LeaderboardNotFound)?; + + if db.is_leaderboard_admin(user.identity.id, lid).await? { + let kmember = db + .get_user_by_name(kick.user.clone()) + .await + .map_err(|_| TimeError::UserNotFound)?; + + db.remove_user_from_leaderboard(lid, kmember.id) + .await + .map_err(|_| TimeError::NotMember)?; + Ok(HttpResponse::Ok().finish()) } else { - error!("{}", TimeError::LeaderboardNotFound); - Err(TimeError::LeaderboardNotFound) + Err(TimeError::Unauthorized) } } @@ -284,16 +247,15 @@ pub async fn regenerate_invite( path: Path<(String,)>, db: DatabaseWrapper, ) -> Result { - if let Ok(lid) = db.get_leaderboard_id_by_name(path.0.clone()).await { - if db.is_leaderboard_admin(user.identity.id, lid).await? { - let code = db.regenerate_leaderboard_invite(lid).await?; - Ok(web::Json(json!({ "invite_code": code }))) - } else { - error!("{}", TimeError::Unauthorized); - Err(TimeError::Unauthorized) - } + let lid = db + .get_leaderboard_id_by_name(path.0.clone()) + .await + .map_err(|_| TimeError::LeaderboardNotFound)?; + + if db.is_leaderboard_admin(user.identity.id, lid).await? { + let code = db.regenerate_leaderboard_invite(lid).await?; + Ok(web::Json(json!({ "invite_code": code }))) } else { - error!("{}", TimeError::LeaderboardNotFound); - Err(TimeError::LeaderboardNotFound) + Err(TimeError::Unauthorized) } } diff --git a/src/api/users.rs b/src/api/users.rs index cfbde0a..9a41cec 100644 --- a/src/api/users.rs +++ b/src/api/users.rs @@ -129,7 +129,7 @@ pub async fn get_current_activity( #[get("/users/{username}/activity/data")] pub async fn get_activities( - data: Query, + Query(data): Query, path: Path<(String,)>, opt_user: UserIdentityOptional, db: DatabaseWrapper, @@ -141,16 +141,14 @@ pub async fn get_activities( .map_err(|_| TimeError::UserNotFound)?; if target_user.is_public { - return Ok(web::Json( - db.get_activity(data.into_inner(), target_user.id).await?, - )); + return Ok(web::Json(db.get_activity(data, target_user.id).await?)); } else { return Err(TimeError::UserNotFound); }; }; let data = if path.0 == "@me" { - db.get_activity(data.into_inner(), user.id).await? + db.get_activity(data, user.id).await? } else { //FIXME: This is technically not required when the username equals the username of the //authenticated user @@ -163,7 +161,7 @@ pub async fn get_activities( || target_user.is_public || db.are_friends(user.id, target_user.id).await? { - db.get_activity(data.into_inner(), target_user.id).await? + db.get_activity(data, target_user.id).await? } else { return Err(TimeError::Unauthorized); } diff --git a/src/main.rs b/src/main.rs index 827895c..a984174 100644 --- a/src/main.rs +++ b/src/main.rs @@ -102,7 +102,6 @@ async fn main() -> std::io::Result<()> { ); let heartbeat_store = Data::new(api::activity::HeartBeatMemoryStore::new()); - let leaderboard_cache = Data::new(api::leaderboards::LeaderboardCache::new()); let secured_access_token_storage = Data::new(SecuredAccessTokenStorage::new()); @@ -182,7 +181,6 @@ async fn main() -> std::io::Result<()> { scope } }) - .app_data(Data::clone(&leaderboard_cache)) .app_data(Data::clone(&database)) .app_data(Data::clone(&heartbeat_store)); #[cfg(feature = "testausid")] diff --git a/src/tests/mod.rs b/src/tests/mod.rs index fa53262..48a8da9 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -32,7 +32,6 @@ fn init_test_services(cfg: &mut ServiceConfig) { ); let heartbeat_store = Data::new(crate::api::activity::HeartBeatMemoryStore::new()); - let leaderboard_cache = Data::new(crate::api::leaderboards::LeaderboardCache::new()); let secured_access_token_storage = Data::new(crate::SecuredAccessTokenStorage::new()); @@ -115,8 +114,7 @@ fn init_test_services(cfg: &mut ServiceConfig) { } }), ) - .app_data(Data::clone(&heartbeat_store)) - .app_data(Data::clone(&leaderboard_cache)); + .app_data(Data::clone(&heartbeat_store)); #[cfg(feature = "testausid")] { cfg.app_data(Data::new(client));