Skip to content

Commit e255072

Browse files
authored
feat: Paginate the federation API calls (#475)
1 parent 5245493 commit e255072

File tree

14 files changed

+388
-27
lines changed

14 files changed

+388
-27
lines changed

Cargo.lock

Lines changed: 42 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ secrecy = { version = "0.10", features = ["serde"] }
6464
serde = { version = "1.0" }
6565
serde_bytes = { version = "0.11" }
6666
serde_json = { version = "1.0" }
67+
serde_urlencoded = { version = "0.7" }
6768
tempfile = { version = "3.23" }
6869
thiserror = { version = "2.0" }
6970
tokio = { version = "1.48", features = ["fs", "macros", "signal", "rt-multi-thread"] }
@@ -90,6 +91,7 @@ hyper-util = { version = "0.1", features = ["tokio", "http1"] }
9091
keycloak = { version = "26.4" }
9192
mockall = { version = "0.14" }
9293
reqwest = { version = "0.12", features = ["json", "multipart"] }
94+
rstest = "0.26"
9395
sea-orm = { version = "1.1", features = ["mock", "sqlx-sqlite" ]}
9496
serde_urlencoded = { version = "0.7" }
9597
thirtyfour = "0.36"

src/api/common.rs

Lines changed: 151 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,12 @@
1212
//
1313
// SPDX-License-Identifier: Apache-2.0
1414
//! # Common API helpers
15-
use crate::api::error::KeystoneApiError;
16-
use crate::api::types::ScopeProject;
15+
use serde::Serialize;
16+
use url::Url;
17+
18+
use crate::api::KeystoneApiError;
19+
use crate::api::types::{Link, ScopeProject};
20+
use crate::config::Config;
1721
use crate::keystone::ServiceState;
1822
use crate::resource::{
1923
ResourceApi,
@@ -117,15 +121,76 @@ pub async fn find_project_from_scope(
117121
Ok(project)
118122
}
119123

124+
/// Prepare the links for the paginated resource collection.
125+
pub fn build_pagination_links<T, Q>(
126+
config: &Config,
127+
data: &[T],
128+
query: &Q,
129+
collection_url: &str,
130+
) -> Result<Option<Vec<Link>>, KeystoneApiError>
131+
where
132+
T: ResourceIdentifier,
133+
Q: QueryParameterPagination + Clone + Serialize,
134+
{
135+
Ok(match &query.get_limit() {
136+
Some(limit) => {
137+
if (data.len() as u64) >= *limit
138+
&& let Some(last_id) = data.last().map(|x| x.get_id())
139+
{
140+
let mut url = Url::parse(
141+
config
142+
.default
143+
.public_endpoint
144+
.as_ref()
145+
.map_or("http://localhost", |v| v),
146+
)?;
147+
url.set_path(collection_url);
148+
let mut new_query = query.clone();
149+
150+
new_query.set_marker(last_id);
151+
url.set_query(Some(&serde_urlencoded::to_string(&new_query)?));
152+
153+
let next_page_url = format!(
154+
"{}{}",
155+
url.path(),
156+
url.query().map(|q| format!("?{}", q)).unwrap_or_default()
157+
);
158+
Some(vec![Link {
159+
rel: String::from("next"),
160+
href: next_page_url,
161+
}])
162+
} else {
163+
None
164+
}
165+
}
166+
None => None,
167+
})
168+
}
169+
170+
/// Resource query parameters pagination extension trait.
171+
pub trait QueryParameterPagination {
172+
/// Get the page limit.
173+
fn get_limit(&self) -> Option<u64>;
174+
/// Set the pagination marker.
175+
fn set_marker(&mut self, marker: String) -> &mut Self;
176+
}
177+
178+
/// Trait for the resource to expose the unique identifier that can be used for building the
179+
/// marker pagination.
180+
pub trait ResourceIdentifier {
181+
/// Get the unique resource identifier.
182+
fn get_id(&self) -> String;
183+
}
184+
120185
#[cfg(test)]
121186
mod tests {
187+
use rstest::rstest;
122188
use sea_orm::DatabaseConnection;
123189
use std::sync::Arc;
124190

125191
use super::*;
126192

127193
use crate::config::Config;
128-
129194
use crate::keystone::Service;
130195
use crate::policy::MockPolicyFactory;
131196
use crate::provider::Provider;
@@ -197,4 +262,87 @@ mod tests {
197262
}
198263
}
199264
}
265+
266+
/// Fake resource for pagination testing
267+
struct FakeResource {
268+
pub id: String,
269+
}
270+
271+
/// Fake query params for pagination testing
272+
#[derive(Clone, Default, Serialize)]
273+
struct FakeQueryParams {
274+
pub marker: Option<String>,
275+
pub limit: Option<u64>,
276+
}
277+
278+
impl ResourceIdentifier for FakeResource {
279+
fn get_id(&self) -> String {
280+
self.id.clone()
281+
}
282+
}
283+
284+
impl QueryParameterPagination for FakeQueryParams {
285+
fn get_limit(&self) -> Option<u64> {
286+
self.limit
287+
}
288+
289+
fn set_marker(&mut self, marker: String) -> &mut Self {
290+
self.marker = Some(marker);
291+
self
292+
}
293+
}
294+
295+
/// Parameterized pagination test
296+
#[rstest]
297+
#[case(5, FakeQueryParams::default(), None)]
298+
#[case(5, FakeQueryParams{marker: Some("x".into()), limit: None}, None)]
299+
#[case(5, FakeQueryParams{marker: Some("x".into()), limit: Some(6)}, None)]
300+
#[case(5, FakeQueryParams{marker: Some("x".into()), limit: Some(5)}, Some(vec![
301+
Link {
302+
rel: "next".into(),
303+
href: "/foo/bar?marker=4&limit=5".into()
304+
}])
305+
)]
306+
#[case(5, FakeQueryParams{marker: Some("x".into()), limit: Some(3)}, Some(vec![
307+
Link {
308+
rel: "next".into(),
309+
href: "/foo/bar?marker=4&limit=3".into()
310+
}])
311+
)]
312+
#[case(5, FakeQueryParams{marker: Some("x".into()), limit: Some(1)}, Some(vec![
313+
Link {
314+
rel: "next".into(),
315+
href: "/foo/bar?marker=4&limit=1".into()
316+
}])
317+
)]
318+
#[case(5, FakeQueryParams{marker: Some("x".into()), limit: Some(0)}, Some(vec![
319+
Link {
320+
rel: "next".into(),
321+
href: "/foo/bar?marker=4&limit=0".into()
322+
}])
323+
)]
324+
#[case(0, FakeQueryParams{marker: Some("x".into()), limit: Some(6)}, None)]
325+
#[case(0, FakeQueryParams{marker: None, limit: Some(6)}, None)]
326+
#[case(5, FakeQueryParams{marker: None, limit: Some(5)}, Some(vec![
327+
Link {
328+
rel: "next".into(),
329+
href: "/foo/bar?marker=4&limit=5".into()
330+
}])
331+
)]
332+
fn test_pagination(
333+
#[case] cnt: usize,
334+
#[case] query: FakeQueryParams,
335+
#[case] expected: Option<Vec<Link>>,
336+
) {
337+
assert_eq!(
338+
build_pagination_links(
339+
&Config::default(),
340+
Vec::from_iter((0..cnt).map(|x| FakeResource { id: x.to_string() })).as_slice(),
341+
&query,
342+
"foo/bar",
343+
)
344+
.unwrap(),
345+
expected
346+
);
347+
}
200348
}

src/api/error.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,18 @@ impl From<BuilderError> for KeystoneApiError {
194194
}
195195
}
196196

197+
impl From<serde_urlencoded::ser::Error> for KeystoneApiError {
198+
fn from(value: serde_urlencoded::ser::Error) -> Self {
199+
Self::InternalError(value.to_string())
200+
}
201+
}
202+
203+
impl From<url::ParseError> for KeystoneApiError {
204+
fn from(value: url::ParseError) -> Self {
205+
Self::InternalError(value.to_string())
206+
}
207+
}
208+
197209
impl From<CatalogProviderError> for KeystoneApiError {
198210
fn from(value: CatalogProviderError) -> Self {
199211
match value {

src/api/mod.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,12 @@ async fn version(
8888
let host = state
8989
.config
9090
.default
91-
.as_ref()
92-
.and_then(|dflt| dflt.public_endpoint.clone())
91+
.public_endpoint
92+
.clone()
9393
.or_else(|| {
9494
headers
9595
.get(header::HOST)
9696
.and_then(|header| header.to_str().map(|val| format!("http://{val}")).ok())
97-
//.and_then(|header| format!("http://{}", header.to_str().ok()).into())
9897
})
9998
.unwrap_or_else(|| "http://localhost".to_string());
10099

src/api/v3/auth/token/types.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ impl TryFrom<UserPassword> for identity_types::UserPasswordAuthRequest {
213213
upa.domain(domain_builder.build()?);
214214
}
215215
upa.password(value.password.clone());
216-
Ok(upa.build()?)
216+
upa.build()
217217
}
218218
}
219219

@@ -246,7 +246,7 @@ impl TryFrom<&BackendToken> for Token {
246246
token.methods(value.methods().clone());
247247
token.audit_ids(value.audit_ids().clone());
248248
token.expires_at(*value.expires_at());
249-
Ok(token.build()?)
249+
token.build()
250250
}
251251
}
252252

src/api/v3/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ async fn version(
6767
let host = state
6868
.config
6969
.default
70-
.as_ref()
71-
.and_then(|dflt| dflt.public_endpoint.clone())
70+
.public_endpoint
71+
.clone()
7272
.or_else(|| {
7373
headers
7474
.get(header::HOST)

src/api/v4/auth/token/types.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ impl TryFrom<UserPassword> for identity_types::UserPasswordAuthRequest {
205205
upa.domain(domain_builder.build()?);
206206
}
207207
upa.password(value.password.clone());
208-
Ok(upa.build()?)
208+
upa.build()
209209
}
210210
}
211211

@@ -238,7 +238,7 @@ impl TryFrom<&BackendToken> for Token {
238238
token.methods(value.methods().clone());
239239
token.audit_ids(value.audit_ids().clone());
240240
token.expires_at(*value.expires_at());
241-
Ok(token.build()?)
241+
token.build()
242242
}
243243
}
244244

src/api/v4/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ async fn version(
7676
let host = state
7777
.config
7878
.default
79-
.as_ref()
80-
.and_then(|dflt| dflt.public_endpoint.clone())
79+
.public_endpoint
80+
.clone()
8181
.or_else(|| {
8282
headers
8383
.get(header::HOST)

src/config.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ use url::Url;
2828
#[derive(Debug, Default, Deserialize, Clone)]
2929
pub struct Config {
3030
/// Global configuration options.
31-
#[serde(rename = "DEFAULT")]
32-
pub default: Option<DefaultSection>,
31+
#[serde(rename = "DEFAULT", default)]
32+
pub default: DefaultSection,
3333

3434
/// Application credentials provider configuration.
3535
#[serde(default)]

0 commit comments

Comments
 (0)