From 5c523009e186ddcc69643f633264b804a9aa6817 Mon Sep 17 00:00:00 2001 From: Natsu Kagami Date: Sun, 4 Aug 2024 21:23:35 +0200 Subject: [PATCH] Avoid updating users when the user id itself changed --- ...c3b787fa284a74b3e3d491755c88a693810e0.json | 20 ++++++++++ youmubot-db-sql/src/models/osu_user.rs | 19 ++++++++- youmubot-osu/src/discord/announcer.rs | 39 +++++++++++-------- youmubot-osu/src/discord/db.rs | 11 ++++-- youmubot-prelude/src/lib.rs | 6 +++ 5 files changed, 73 insertions(+), 22 deletions(-) create mode 100644 youmubot-db-sql/.sqlx/query-9c620436cfe463780fca2dc5497c3b787fa284a74b3e3d491755c88a693810e0.json diff --git a/youmubot-db-sql/.sqlx/query-9c620436cfe463780fca2dc5497c3b787fa284a74b3e3d491755c88a693810e0.json b/youmubot-db-sql/.sqlx/query-9c620436cfe463780fca2dc5497c3b787fa284a74b3e3d491755c88a693810e0.json new file mode 100644 index 0000000..f5ac4eb --- /dev/null +++ b/youmubot-db-sql/.sqlx/query-9c620436cfe463780fca2dc5497c3b787fa284a74b3e3d491755c88a693810e0.json @@ -0,0 +1,20 @@ +{ + "db_name": "SQLite", + "query": "SELECT id as \"id: i64\" FROM osu_users WHERE user_id = ?", + "describe": { + "columns": [ + { + "name": "id: i64", + "ordinal": 0, + "type_info": "Int64" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + false + ] + }, + "hash": "9c620436cfe463780fca2dc5497c3b787fa284a74b3e3d491755c88a693810e0" +} diff --git a/youmubot-db-sql/src/models/osu_user.rs b/youmubot-db-sql/src/models/osu_user.rs index 74af102..9ac6c0a 100644 --- a/youmubot-db-sql/src/models/osu_user.rs +++ b/youmubot-db-sql/src/models/osu_user.rs @@ -182,7 +182,22 @@ impl OsuUser { impl OsuUser { /// Stores the user. - pub async fn store<'a>(&self, conn: &mut Transaction<'a, Database>) -> Result<()> { + pub async fn store<'a>(&self, conn: &mut Transaction<'a, Database>) -> Result { + let old_user_id = { + query!( + r#"SELECT id as "id: i64" FROM osu_users WHERE user_id = ?"#, + self.user_id + ) + .fetch_optional(&mut **conn) + .await? + .map(|v| v.id) + }; + + if old_user_id.is_some_and(|v| v != self.id) { + // There's another update that changed the user_id + return Ok(false); + } + query!( r#"INSERT INTO osu_users(user_id, username, id, failures) @@ -221,7 +236,7 @@ impl OsuUser { .execute(&mut **conn) .await?; } - Ok(()) + Ok(true) } pub async fn delete(user_id: i64, conn: impl Executor<'_, Database = Database>) -> Result<()> { diff --git a/youmubot-osu/src/discord/announcer.rs b/youmubot-osu/src/discord/announcer.rs index abc7d08..7c36144 100644 --- a/youmubot-osu/src/discord/announcer.rs +++ b/youmubot-osu/src/discord/announcer.rs @@ -1,5 +1,7 @@ use chrono::{DateTime, Utc}; +use future::Future; use futures_util::try_join; +use std::pin::Pin; use std::sync::Arc; use stream::FuturesUnordered; @@ -57,17 +59,11 @@ impl youmubot_prelude::Announcer for Announcer { let users = env.saved_users.all().await?; users .into_iter() - .map( - |osu_user| { - channels - .channels_of(ctx.clone(), osu_user.user_id) - .then(|chs| self.update_user(ctx.clone(), &env, osu_user, chs)) - .then(|new_user| env.saved_users.save(new_user)) - .map(|r| { - r.pls_ok(); - }) - }, // self.update_user() - ) + .map(|osu_user| { + channels + .channels_of(ctx.clone(), osu_user.user_id) + .then(|chs| self.update_user(ctx.clone(), &env, osu_user, chs)) + }) .collect::>() .collect::<()>() .await; @@ -82,13 +78,17 @@ impl Announcer { env: &OsuEnv, mut user: OsuUser, broadcast_to: Vec, - ) -> OsuUser { + ) { + if broadcast_to.is_empty() { + return; // Skip update if there are no broadcasting channels + } if user.failures == MAX_FAILURES { - return user; + return; } const MODES: [Mode; 4] = [Mode::Std, Mode::Taiko, Mode::Catch, Mode::Mania]; let now = chrono::Utc::now(); let broadcast_to = Arc::new(broadcast_to); + let mut to_announce = Vec:: + Send>>>::new(); for mode in MODES { let (u, top, events) = match self.fetch_user_data(env, now, &user, mode).await { Ok(v) => v, @@ -125,7 +125,7 @@ impl Announcer { let ctx = ctx.clone(); let env = env.clone(); if let Some(last) = last { - spawn_future(async move { + to_announce.push(Box::pin(async move { let top = top .into_iter() .enumerate() @@ -147,11 +147,18 @@ impl Announcer { .filter_map(|v| future::ready(v.pls_ok().map(|_| ()))) .collect::<()>() .await - }); + })); } } user.failures = 0; - user + let user_id = user.user_id; + if let Some(true) = env.saved_users.save(user).await.pls_ok() { + to_announce.into_iter().for_each(|v| { + spawn_future(v); + }); + } else { + eprintln!("[osu] Skipping user {} due to raced update", user_id) + } } fn is_announceable_date( diff --git a/youmubot-osu/src/discord/db.rs b/youmubot-osu/src/discord/db.rs index 3323f83..a828af7 100644 --- a/youmubot-osu/src/discord/db.rs +++ b/youmubot-osu/src/discord/db.rs @@ -46,18 +46,21 @@ impl OsuSavedUsers { } /// Save the given user. - pub async fn save(&self, u: OsuUser) -> Result<()> { + pub async fn save(&self, u: OsuUser) -> Result { let mut tx = self.pool.begin().await?; - model::OsuUser::from(u).store(&mut tx).await?; + let updated = model::OsuUser::from(u).store(&mut tx).await?; tx.commit().await?; - Ok(()) + Ok(updated) } /// Save the given user as a completely new user. pub async fn new_user(&self, u: OsuUser) -> Result<()> { let mut t = self.pool.begin().await?; model::OsuUser::delete(u.user_id.get() as i64, &mut *t).await?; - model::OsuUser::from(u).store(&mut t).await?; + assert!( + model::OsuUser::from(u).store(&mut t).await?, + "Should be updated" + ); t.commit().await?; Ok(()) } diff --git a/youmubot-prelude/src/lib.rs b/youmubot-prelude/src/lib.rs index 6e4683d..76f03bf 100644 --- a/youmubot-prelude/src/lib.rs +++ b/youmubot-prelude/src/lib.rs @@ -88,6 +88,8 @@ pub mod prelude_commands { } mod debugging_ok { + use std::backtrace::{Backtrace, BacktraceStatus}; + pub trait OkPrint { type Output; fn pls_ok(self) -> Option; @@ -101,6 +103,10 @@ mod debugging_ok { Ok(v) => Some(v), Err(e) => { eprintln!("Error: {:?}", e); + let captures = Backtrace::capture(); + if captures.status() == BacktraceStatus::Captured { + eprintln!("{}", captures); + } None } }