Skip to content

Commit 4926325

Browse files
authored
models/team: Avoid repeated AccessToken instantiation (#10589)
We don't need a new instance for each request...
1 parent 8756a88 commit 4926325

File tree

2 files changed

+21
-17
lines changed

2 files changed

+21
-17
lines changed

src/models/team.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ impl Team {
138138

139139
let org_id = team.organization.id;
140140

141-
if !can_add_team(gh_client, org_id, team.id, req_user).await? {
141+
if !can_add_team(gh_client, org_id, team.id, &req_user.gh_login, &token).await? {
142142
return Err(custom(
143143
StatusCode::FORBIDDEN,
144144
"only members of a team or organization owners can add it as an owner",
@@ -166,11 +166,13 @@ impl Team {
166166
pub async fn contains_user(
167167
&self,
168168
gh_client: &dyn GitHubClient,
169-
user: &User,
169+
gh_login: &str,
170+
token: &AccessToken,
170171
) -> Result<bool, GitHubError> {
171172
match self.org_id {
172173
Some(org_id) => {
173-
team_with_gh_id_contains_user(gh_client, org_id, self.github_id, user).await
174+
team_with_gh_id_contains_user(gh_client, org_id, self.github_id, gh_login, token)
175+
.await
174176
}
175177
// This means we don't have an org_id on file for the `self` team. It much
176178
// probably was deleted from github by the time we backfilled the database.
@@ -199,39 +201,37 @@ async fn can_add_team(
199201
gh_client: &dyn GitHubClient,
200202
org_id: i32,
201203
team_id: i32,
202-
user: &User,
204+
gh_login: &str,
205+
token: &AccessToken,
203206
) -> Result<bool, GitHubError> {
204207
Ok(
205-
team_with_gh_id_contains_user(gh_client, org_id, team_id, user).await?
206-
|| is_gh_org_owner(gh_client, org_id, user).await?,
208+
team_with_gh_id_contains_user(gh_client, org_id, team_id, gh_login, token).await?
209+
|| is_gh_org_owner(gh_client, org_id, gh_login, token).await?,
207210
)
208211
}
209212

210213
async fn is_gh_org_owner(
211214
gh_client: &dyn GitHubClient,
212215
org_id: i32,
213-
user: &User,
216+
gh_login: &str,
217+
token: &AccessToken,
214218
) -> Result<bool, GitHubError> {
215-
let token = AccessToken::new(user.gh_access_token.expose_secret().to_string());
216-
let membership = gh_client
217-
.org_membership(org_id, &user.gh_login, &token)
218-
.await?;
219-
219+
let membership = gh_client.org_membership(org_id, gh_login, token).await?;
220220
Ok(membership.is_some_and(|m| m.state == "active" && m.role == "admin"))
221221
}
222222

223223
async fn team_with_gh_id_contains_user(
224224
gh_client: &dyn GitHubClient,
225225
github_org_id: i32,
226226
github_team_id: i32,
227-
user: &User,
227+
gh_login: &str,
228+
token: &AccessToken,
228229
) -> Result<bool, GitHubError> {
229230
// GET /organizations/:org_id/team/:team_id/memberships/:username
230231
// check that "state": "active"
231232

232-
let token = AccessToken::new(user.gh_access_token.expose_secret().to_string());
233233
let membership = gh_client
234-
.team_membership(github_org_id, github_team_id, &user.gh_login, &token)
234+
.team_membership(github_org_id, github_team_id, gh_login, token)
235235
.await?;
236236

237237
// There is also `state: pending` for which we could possibly give

src/models/user.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ use diesel::prelude::*;
55
use diesel::sql_types::Integer;
66
use diesel::upsert::excluded;
77
use diesel_async::{AsyncPgConnection, RunQueryDsl};
8-
use secrecy::SecretString;
8+
use oauth2::AccessToken;
9+
use secrecy::{ExposeSecret, SecretString};
910

1011
use crate::models::{Crate, CrateOwner, Email, Owner, OwnerKind, Rights};
1112
use crate::schema::{crate_owners, emails, users};
@@ -68,6 +69,8 @@ impl User {
6869
gh_client: &dyn GitHubClient,
6970
owners: &[Owner],
7071
) -> Result<Rights, GitHubError> {
72+
let token = AccessToken::new(self.gh_access_token.expose_secret().to_string());
73+
7174
let mut best = Rights::None;
7275
for owner in owners {
7376
match *owner {
@@ -77,7 +80,8 @@ impl User {
7780
}
7881
}
7982
Owner::Team(ref team) => {
80-
if team.contains_user(gh_client, self).await? {
83+
let gh_login = &self.gh_login;
84+
if team.contains_user(gh_client, gh_login, &token).await? {
8185
best = Rights::Publish;
8286
}
8387
}

0 commit comments

Comments
 (0)