Skip to content

Commit bde63dd

Browse files
committed
Use the list function with a fixed amount of results instead of a custom function
Signed-off-by: MTRNord <[email protected]>
1 parent 0eac024 commit bde63dd

File tree

3 files changed

+18
-72
lines changed

3 files changed

+18
-72
lines changed

crates/handlers/src/graphql/model/upstream_oauth.rs

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@
77
use anyhow::Context as _;
88
use async_graphql::{Context, Object, ID};
99
use chrono::{DateTime, Utc};
10-
use mas_storage::{upstream_oauth2::UpstreamOAuthProviderRepository, user::UserRepository};
10+
use mas_storage::{
11+
upstream_oauth2::{UpstreamOAuthLinkFilter, UpstreamOAuthProviderRepository},
12+
user::UserRepository,
13+
Pagination,
14+
};
1115

1216
use super::{NodeType, User};
1317
use crate::graphql::state::ContextExt;
@@ -57,20 +61,28 @@ impl UpstreamOAuth2Provider {
5761
ctx: &Context<'_>,
5862
) -> Result<Vec<UpstreamOAuth2Link>, async_graphql::Error> {
5963
let state = ctx.state();
60-
let user_id = ctx
64+
let user = ctx
6165
.requester()
6266
.user()
63-
.ok_or_else(|| async_graphql::Error::new("User ID not found in the request context"))?
64-
.id;
67+
.ok_or_else(|| async_graphql::Error::new("User ID not found in the request context"))?;
6568

6669
let mut repo = state.repository().await?;
70+
let filter = UpstreamOAuthLinkFilter::new()
71+
.for_provider(&self.provider)
72+
.for_user(&user);
6773
let links = repo
6874
.upstream_oauth_link()
69-
.find_by_user_id(&self.provider, user_id)
75+
// Hardcoded limit of 100 links. We do not expect reasonably more links
76+
// See also https://github.com/element-hq/matrix-authentication-service/pull/3245#discussion_r1776850096
77+
.list(filter, Pagination::first(100))
7078
.await?;
7179
repo.cancel().await?;
7280

73-
Ok(links.into_iter().map(UpstreamOAuth2Link::new).collect())
81+
Ok(links
82+
.edges
83+
.into_iter()
84+
.map(UpstreamOAuth2Link::new)
85+
.collect())
7486
}
7587
}
7688

crates/storage-pg/src/upstream_oauth2/link.rs

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -179,47 +179,6 @@ impl<'c> UpstreamOAuthLinkRepository for PgUpstreamOAuthLinkRepository<'c> {
179179
Ok(res)
180180
}
181181

182-
#[tracing::instrument(
183-
name = "db.upstream_oauth_link.find_by_user_id",
184-
skip_all,
185-
fields(
186-
db.query.text,
187-
upstream_oauth_link.user_id = user_id.0,
188-
%upstream_oauth_provider.id,
189-
%upstream_oauth_provider.issuer,
190-
%upstream_oauth_provider.client_id,
191-
),
192-
err,
193-
)]
194-
async fn find_by_user_id(
195-
&mut self,
196-
upstream_oauth_provider: &UpstreamOAuthProvider,
197-
user_id: Ulid,
198-
) -> Result<Option<UpstreamOAuthLink>, Self::Error> {
199-
let res = sqlx::query_as!(
200-
LinkLookup,
201-
r#"
202-
SELECT
203-
upstream_oauth_link_id,
204-
upstream_oauth_provider_id,
205-
user_id,
206-
subject,
207-
created_at
208-
FROM upstream_oauth_links
209-
WHERE upstream_oauth_provider_id = $1
210-
AND user_id = $2
211-
"#,
212-
Uuid::from(upstream_oauth_provider.id),
213-
Uuid::from(user_id),
214-
)
215-
.traced()
216-
.fetch_optional(&mut *self.conn)
217-
.await?
218-
.map(Into::into);
219-
220-
Ok(res)
221-
}
222-
223182
#[tracing::instrument(
224183
name = "db.upstream_oauth_link.add",
225184
skip_all,

crates/storage/src/upstream_oauth2/link.rs

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -117,25 +117,6 @@ pub trait UpstreamOAuthLinkRepository: Send + Sync {
117117
subject: &str,
118118
) -> Result<Option<UpstreamOAuthLink>, Self::Error>;
119119

120-
/// Find an upstream OAuth link for a provider by the associated user id
121-
///
122-
/// Returns `None` if no matching upstream OAuth link was found
123-
///
124-
/// # Parameters
125-
///
126-
/// * `upstream_oauth_provider`: The upstream OAuth provider on which to
127-
/// find the link
128-
/// * `user_id`: The user id of the upstream OAuth link to find
129-
///
130-
/// # Errors
131-
///
132-
/// Returns [`Self::Error`] if the underlying repository fails
133-
async fn find_by_user_id(
134-
&mut self,
135-
upstream_oauth_provider: &UpstreamOAuthProvider,
136-
user_id: Ulid,
137-
) -> Result<Option<UpstreamOAuthLink>, Self::Error>;
138-
139120
/// Add a new upstream OAuth link
140121
///
141122
/// Returns the newly created upstream OAuth link
@@ -214,12 +195,6 @@ repository_impl!(UpstreamOAuthLinkRepository:
214195
subject: &str,
215196
) -> Result<Option<UpstreamOAuthLink>, Self::Error>;
216197

217-
async fn find_by_user_id(
218-
&mut self,
219-
upstream_oauth_provider: &UpstreamOAuthProvider,
220-
user_id: Ulid,
221-
) -> Result<Option<UpstreamOAuthLink>, Self::Error>;
222-
223198
async fn add(
224199
&mut self,
225200
rng: &mut (dyn RngCore + Send),

0 commit comments

Comments
 (0)