Skip to content

Commit

Permalink
Merge pull request #1771 from WaffleLapkin/usizeless
Browse files Browse the repository at this point in the history
Don't use `usize` for ids and such
  • Loading branch information
ehuss authored Feb 14, 2024
2 parents b0bbe70 + 60e608e commit c5cd297
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 78 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 20 additions & 17 deletions src/db/notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use tokio_postgres::Client as DbClient;
use tracing as log;

pub struct Notification {
pub user_id: i64,
pub user_id: u64,
pub origin_url: String,
pub origin_html: String,
pub short_description: Option<String>,
Expand All @@ -15,10 +15,10 @@ pub struct Notification {
pub team_name: Option<String>,
}

pub async fn record_username(db: &DbClient, user_id: i64, username: String) -> anyhow::Result<()> {
pub async fn record_username(db: &DbClient, user_id: u64, username: String) -> anyhow::Result<()> {
db.execute(
"INSERT INTO users (user_id, username) VALUES ($1, $2) ON CONFLICT DO NOTHING",
&[&user_id, &username],
&[&(user_id as i64), &username],
)
.await
.context("inserting user id / username")?;
Expand All @@ -31,7 +31,7 @@ pub async fn record_ping(db: &DbClient, notification: &Notification) -> anyhow::
$1, $2, $3, $4, $5, $6,
(SELECT max(notifications.idx) + 1 from notifications where notifications.user_id = $1)
)",
&[&notification.user_id, &notification.origin_url, &notification.origin_html, &notification.time, &notification.short_description, &notification.team_name],
&[&(notification.user_id as i64), &notification.origin_url, &notification.origin_html, &notification.time, &notification.short_description, &notification.team_name],
).await.context("inserting notification")?;

Ok(())
Expand All @@ -40,14 +40,14 @@ pub async fn record_ping(db: &DbClient, notification: &Notification) -> anyhow::
#[derive(Copy, Clone)]
pub enum Identifier<'a> {
Url(&'a str),
Index(std::num::NonZeroUsize),
Index(std::num::NonZeroU32),
/// Glob identifier (`all` or `*`).
All,
}

pub async fn delete_ping(
db: &mut DbClient,
user_id: i64,
user_id: u64,
identifier: Identifier<'_>,
) -> anyhow::Result<Vec<NotificationData>> {
match identifier {
Expand All @@ -56,7 +56,7 @@ pub async fn delete_ping(
.query(
"DELETE FROM notifications WHERE user_id = $1 and origin_url = $2
RETURNING origin_html, time, short_description, metadata",
&[&user_id, &origin_url],
&[&(user_id as i64), &origin_url],
)
.await
.context("delete notification query")?;
Expand Down Expand Up @@ -91,13 +91,13 @@ pub async fn delete_ping(
from notifications
where user_id = $1
order by idx asc nulls last;",
&[&user_id],
&[&(user_id as i64)],
)
.await
.context("failed to get ordering")?;

let notification_id: i64 = notifications
.get(idx.get() - 1)
.get((idx.get() - 1) as usize)
.ok_or_else(|| anyhow::anyhow!("No such notification with index {}", idx.get()))?
.get(0);

Expand Down Expand Up @@ -144,7 +144,7 @@ pub async fn delete_ping(
.query(
"DELETE FROM notifications WHERE user_id = $1
RETURNING origin_url, origin_html, time, short_description, metadata",
&[&user_id],
&[&(user_id as i64)],
)
.await
.context("delete all notifications query")?;
Expand Down Expand Up @@ -180,10 +180,12 @@ pub struct NotificationData {

pub async fn move_indices(
db: &mut DbClient,
user_id: i64,
from: usize,
to: usize,
user_id: u64,
from: u32,
to: u32,
) -> anyhow::Result<()> {
let from = usize::try_from(from)?;
let to = usize::try_from(to)?;
loop {
let t = db
.build_transaction()
Expand All @@ -198,7 +200,7 @@ pub async fn move_indices(
from notifications
where user_id = $1
order by idx asc nulls last;",
&[&user_id],
&[&(user_id as i64)],
)
.await
.context("failed to get initial ordering")?;
Expand Down Expand Up @@ -257,10 +259,11 @@ pub async fn move_indices(

pub async fn add_metadata(
db: &mut DbClient,
user_id: i64,
idx: usize,
user_id: u64,
idx: u32,
metadata: Option<&str>,
) -> anyhow::Result<()> {
let idx = usize::try_from(idx)?;
loop {
let t = db
.build_transaction()
Expand All @@ -275,7 +278,7 @@ pub async fn add_metadata(
from notifications
where user_id = $1
order by idx asc nulls last;",
&[&user_id],
&[&(user_id as i64)],
)
.await
.context("failed to get initial ordering")?;
Expand Down
18 changes: 9 additions & 9 deletions src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ use tracing as log;
#[derive(Debug, PartialEq, Eq, serde::Deserialize)]
pub struct User {
pub login: String,
pub id: Option<i64>,
pub id: Option<u64>,
}

impl GithubClient {
async fn send_req(&self, req: RequestBuilder) -> anyhow::Result<(Bytes, String)> {
const MAX_ATTEMPTS: usize = 2;
const MAX_ATTEMPTS: u32 = 2;
log::debug!("send_req with {:?}", req);
let req_dbg = format!("{:?}", req);
let req = req
Expand Down Expand Up @@ -80,7 +80,7 @@ impl GithubClient {
&self,
req: Request,
sleep: Duration,
remaining_attempts: usize,
remaining_attempts: u32,
) -> BoxFuture<Result<Response, reqwest::Error>> {
#[derive(Debug, serde::Deserialize)]
struct RateLimit {
Expand Down Expand Up @@ -203,7 +203,7 @@ impl User {
}

// Returns the ID of the given user, if the user is in the `all` team.
pub async fn get_id<'a>(&'a self, client: &'a GithubClient) -> anyhow::Result<Option<usize>> {
pub async fn get_id<'a>(&'a self, client: &'a GithubClient) -> anyhow::Result<Option<u64>> {
let permission = crate::team_data::teams(client).await?;
let map = permission.teams;
Ok(map["all"]
Expand Down Expand Up @@ -504,7 +504,7 @@ impl Issue {
self.state == IssueState::Open
}

pub async fn get_comment(&self, client: &GithubClient, id: usize) -> anyhow::Result<Comment> {
pub async fn get_comment(&self, client: &GithubClient, id: u64) -> anyhow::Result<Comment> {
let comment_url = format!("{}/issues/comments/{}", self.repository().url(client), id);
let comment = client.json(client.get(&comment_url)).await?;
Ok(comment)
Expand Down Expand Up @@ -540,7 +540,7 @@ impl Issue {
pub async fn edit_comment(
&self,
client: &GithubClient,
id: usize,
id: u64,
new_body: &str,
) -> anyhow::Result<()> {
let comment_url = format!("{}/issues/comments/{}", self.repository().url(client), id);
Expand Down Expand Up @@ -1030,7 +1030,7 @@ pub fn parse_diff(diff: &str) -> Vec<FileDiff> {

#[derive(Debug, serde::Deserialize)]
pub struct IssueSearchResult {
pub total_count: usize,
pub total_count: u64,
pub incomplete_results: bool,
pub items: Vec<Issue>,
}
Expand All @@ -1049,7 +1049,7 @@ struct Ordering<'a> {
pub sort: &'a str,
pub direction: &'a str,
pub per_page: &'a str,
pub page: usize,
pub page: u64,
}

impl Repository {
Expand Down Expand Up @@ -1136,7 +1136,7 @@ impl Repository {
.await
.with_context(|| format!("failed to list issues from {}", url))?;
issues.extend(result.items);
if issues.len() < result.total_count {
if (issues.len() as u64) < result.total_count {
ordering.page += 1;
continue;
}
Expand Down
1 change: 1 addition & 0 deletions src/handlers/assign/tests/tests_candidates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ fn convert_simplified(
"members": serde_json::Value::Array(members),
"alumni": [],
"discord": [],
"roles": [],
});
}
let teams = serde_json::value::from_value(teams_config).unwrap();
Expand Down
46 changes: 14 additions & 32 deletions src/handlers/notification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
};
use anyhow::Context as _;
use std::collections::HashSet;
use std::convert::{TryFrom, TryInto};
use std::convert::TryInto;
use tracing as log;

pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> {
Expand Down Expand Up @@ -58,7 +58,7 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> {
let mut users_notified = HashSet::new();
if let Some(from) = event.comment_from() {
for login in parser::get_mentions(from).into_iter() {
if let Some((Ok(users), _)) = id_from_user(ctx, login).await? {
if let Some((users, _)) = id_from_user(ctx, login).await? {
users_notified.extend(users.into_iter().map(|user| user.id.unwrap()));
}
}
Expand All @@ -85,14 +85,6 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> {
None => continue,
};

let users = match users {
Ok(users) => users,
Err(err) => {
log::error!("getting users failed: {:?}", err);
continue;
}
};

let client = ctx.db.get().await;
for user in users {
if !users_notified.insert(user.id.unwrap()) {
Expand Down Expand Up @@ -132,7 +124,7 @@ pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> {
async fn id_from_user(
ctx: &Context,
login: &str,
) -> anyhow::Result<Option<(anyhow::Result<Vec<github::User>>, Option<String>)>> {
) -> anyhow::Result<Option<(Vec<github::User>, Option<String>)>> {
if login.contains('/') {
// This is a team ping. For now, just add it to everyone's agenda on
// that team, but also mark it as such (i.e., a team ping) for
Expand Down Expand Up @@ -174,15 +166,11 @@ async fn id_from_user(
Ok(Some((
team.members
.into_iter()
.map(|member| {
let id = i64::try_from(member.github_id)
.with_context(|| format!("user id {} out of bounds", member.github_id))?;
Ok(github::User {
id: Some(id),
login: member.github,
})
.map(|member| github::User {
id: Some(member.github_id),
login: member.github,
})
.collect::<anyhow::Result<Vec<github::User>>>(),
.collect::<Vec<github::User>>(),
Some(team.name),
)))
} else {
Expand All @@ -194,22 +182,16 @@ async fn id_from_user(
.get_id(&ctx.github)
.await
.with_context(|| format!("failed to get user {} ID", user.login))?;
let id = match id {
Some(id) => id,
let Some(id) = id else {
// If the user was not in the team(s) then just don't record it.
None => {
log::trace!("Skipping {} because no id found", user.login);
return Ok(None);
}
log::trace!("Skipping {} because no id found", user.login);
return Ok(None);
};
let id = i64::try_from(id).with_context(|| format!("user id {} out of bounds", id));
Ok(Some((
id.map(|id| {
vec![github::User {
login: user.login.clone(),
id: Some(id),
}]
}),
vec![github::User {
login: user.login.clone(),
id: Some(id),
}],
None,
)))
}
Expand Down
2 changes: 1 addition & 1 deletion src/handlers/rustc_commits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use std::collections::VecDeque;
use std::convert::TryInto;
use tracing as log;

const BORS_GH_ID: i64 = 3372342;
const BORS_GH_ID: u64 = 3372342;

pub async fn handle(ctx: &Context, event: &Event) -> anyhow::Result<()> {
let body = match event.comment_body() {
Expand Down
Loading

0 comments on commit c5cd297

Please sign in to comment.