From a76526753e932fb0882ac334197da5f9c78a91e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Feb 2025 09:10:38 +0100 Subject: [PATCH 1/3] Move user query to `db/users.rs` --- src/db.rs | 1 + src/db/notifications.rs | 11 ----------- src/db/users.rs | 13 +++++++++++++ src/handlers/notification.rs | 4 ++-- src/handlers/pr_tracking.rs | 2 +- src/handlers/pull_requests_assignment_update.rs | 2 +- src/tests/mod.rs | 2 +- 7 files changed, 19 insertions(+), 16 deletions(-) create mode 100644 src/db/users.rs diff --git a/src/db.rs b/src/db.rs index 43bb7df59..28c9cd32f 100644 --- a/src/db.rs +++ b/src/db.rs @@ -11,6 +11,7 @@ pub mod issue_data; pub mod jobs; pub mod notifications; pub mod rustc_commits; +pub mod users; const CERT_URL: &str = "https://truststore.pki.rds.amazonaws.com/global/global-bundle.pem"; diff --git a/src/db/notifications.rs b/src/db/notifications.rs index 1e9f295ac..3077ef13a 100644 --- a/src/db/notifications.rs +++ b/src/db/notifications.rs @@ -15,17 +15,6 @@ pub struct Notification { pub team_name: Option, } -/// Add a new user (if not existing) -pub async fn record_username(db: &DbClient, user_id: u64, username: &str) -> anyhow::Result<()> { - db.execute( - "INSERT INTO users (user_id, username) VALUES ($1, $2) ON CONFLICT DO NOTHING", - &[&(user_id as i64), &username], - ) - .await - .context("inserting user id / username")?; - Ok(()) -} - pub async fn record_ping(db: &DbClient, notification: &Notification) -> anyhow::Result<()> { db.execute("INSERT INTO notifications (user_id, origin_url, origin_html, time, short_description, team_name, idx) VALUES ( diff --git a/src/db/users.rs b/src/db/users.rs new file mode 100644 index 000000000..979ba6a9e --- /dev/null +++ b/src/db/users.rs @@ -0,0 +1,13 @@ +use anyhow::Context; +use tokio_postgres::Client as DbClient; + +/// Add a new user (if not existing) +pub async fn record_username(db: &DbClient, user_id: u64, username: &str) -> anyhow::Result<()> { + db.execute( + "INSERT INTO users (user_id, username) VALUES ($1, $2) ON CONFLICT DO NOTHING", + &[&(user_id as i64), &username], + ) + .await + .context("inserting user id / username")?; + Ok(()) +} diff --git a/src/handlers/notification.rs b/src/handlers/notification.rs index 87fe20539..e33eb9568 100644 --- a/src/handlers/notification.rs +++ b/src/handlers/notification.rs @@ -4,7 +4,7 @@ //! //! Parsing is done in the `parser::command::ping` module. -use crate::db::notifications; +use crate::db::{notifications, users}; use crate::github::get_id_for_username; use crate::{ github::{self, Event}, @@ -92,7 +92,7 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> { continue; } - if let Err(err) = notifications::record_username(&client, user.id, &user.login) + if let Err(err) = users::record_username(&client, user.id, &user.login) .await .context("failed to record username") { diff --git a/src/handlers/pr_tracking.rs b/src/handlers/pr_tracking.rs index fb1c9bd0c..f793fb128 100644 --- a/src/handlers/pr_tracking.rs +++ b/src/handlers/pr_tracking.rs @@ -7,10 +7,10 @@ //! - Removes the PR from the workqueue of one team member (after the PR has been unassigned or closed) use super::assign::{FindReviewerError, REVIEWER_HAS_NO_CAPACITY, SELF_ASSIGN_HAS_NO_CAPACITY}; +use crate::db::users::record_username; use crate::github::User; use crate::{ config::ReviewPrefsConfig, - db::notifications::record_username, github::{IssuesAction, IssuesEvent}, handlers::Context, ReviewPrefs, diff --git a/src/handlers/pull_requests_assignment_update.rs b/src/handlers/pull_requests_assignment_update.rs index fe544afeb..c5e6acef9 100644 --- a/src/handlers/pull_requests_assignment_update.rs +++ b/src/handlers/pull_requests_assignment_update.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use crate::db::notifications::record_username; +use crate::db::users::record_username; use crate::github::retrieve_pull_requests; use crate::jobs::Job; use crate::ReviewPrefs; diff --git a/src/tests/mod.rs b/src/tests/mod.rs index adfb7c3fa..e24a21251 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -1,5 +1,5 @@ use crate::db; -use crate::db::notifications::record_username; +use crate::db::users::record_username; use crate::db::{make_client, ClientPool, PooledClient}; use crate::github::GithubClient; use crate::handlers::Context; From d62132e10322fb0b5615e160df415a3e36e68f35 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Feb 2025 09:15:02 +0100 Subject: [PATCH 2/3] Update username when recording user in the DB As usernames can change over time. --- src/db/users.rs | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/db/users.rs b/src/db/users.rs index 979ba6a9e..7c97f5629 100644 --- a/src/db/users.rs +++ b/src/db/users.rs @@ -4,10 +4,39 @@ use tokio_postgres::Client as DbClient; /// Add a new user (if not existing) pub async fn record_username(db: &DbClient, user_id: u64, username: &str) -> anyhow::Result<()> { db.execute( - "INSERT INTO users (user_id, username) VALUES ($1, $2) ON CONFLICT DO NOTHING", + r" +INSERT INTO users (user_id, username) VALUES ($1, $2) +ON CONFLICT (user_id) +DO UPDATE SET username = $2", &[&(user_id as i64), &username], ) .await .context("inserting user id / username")?; Ok(()) } + +#[cfg(test)] +mod tests { + use crate::db::users::record_username; + use crate::tests::run_test; + + #[tokio::test] + async fn update_username_on_conflict() { + run_test(|ctx| async { + let db = ctx.db_client().await; + + record_username(&db, 1, "Foo").await?; + record_username(&db, 1, "Bar").await?; + + let row = db + .query_one("SELECT username FROM users WHERE user_id = 1", &[]) + .await + .unwrap(); + let name: &str = row.get(0); + assert_eq!(name, "Bar"); + + Ok(ctx) + }) + .await; + } +} From c7f47c36c09cf9d4d8c4b17ecbdd7b8e4217cfea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 20 Feb 2025 09:26:48 +0100 Subject: [PATCH 3/3] Add a function for loading a user from the DB --- src/db/users.rs | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/db/users.rs b/src/db/users.rs index 7c97f5629..ae8d6efe2 100644 --- a/src/db/users.rs +++ b/src/db/users.rs @@ -1,7 +1,9 @@ +use crate::github::User; use anyhow::Context; use tokio_postgres::Client as DbClient; -/// Add a new user (if not existing) +/// Add a new user. +/// If an user already exists, updates their username. pub async fn record_username(db: &DbClient, user_id: u64, username: &str) -> anyhow::Result<()> { db.execute( r" @@ -15,9 +17,30 @@ DO UPDATE SET username = $2", Ok(()) } +/// Return a user from the DB. +pub async fn get_user(db: &DbClient, user_id: u64) -> anyhow::Result> { + let row = db + .query_opt( + r" +SELECT username +FROM users +WHERE user_id = $1;", + &[&(user_id as i64)], + ) + .await + .context("cannot load user from DB")?; + Ok(row.map(|row| { + let username: &str = row.get(0); + User { + id: user_id, + login: username.to_string(), + } + })) +} + #[cfg(test)] mod tests { - use crate::db::users::record_username; + use crate::db::users::{get_user, record_username}; use crate::tests::run_test; #[tokio::test] @@ -28,12 +51,7 @@ mod tests { record_username(&db, 1, "Foo").await?; record_username(&db, 1, "Bar").await?; - let row = db - .query_one("SELECT username FROM users WHERE user_id = 1", &[]) - .await - .unwrap(); - let name: &str = row.get(0); - assert_eq!(name, "Bar"); + assert_eq!(get_user(&db, 1).await?.unwrap().login, "Bar"); Ok(ctx) })