From b6b54d2c05a3c9a8a6108b277dd43c02f1cd441b Mon Sep 17 00:00:00 2001 From: Marco Napetti Date: Mon, 6 Feb 2023 09:06:23 +0100 Subject: [PATCH 1/2] Paginate versions only when per_page parameter is present --- src/controllers/krate/metadata.rs | 47 +++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index e0f85892c62..bfe30674c3f 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -8,7 +8,8 @@ use std::cmp::Reverse; use std::str::FromStr; use crate::controllers::frontend_prelude::*; -use crate::controllers::helpers::pagination::PaginationOptions; +use crate::controllers::helpers::Paginate; +use crate::controllers::helpers::pagination::{PaginationOptions, Paginated}; use crate::models::{ Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads, @@ -320,15 +321,34 @@ pub async fn readme( /// Handles the `GET /crates/:crate_id/versions` route. // FIXME: Not sure why this is necessary since /crates/:crate_id returns // this information already, but ember is definitely requesting it -pub async fn versions(state: AppState, Path(crate_name): Path) -> AppResult> { +pub async fn versions(state: AppState, Path(crate_name): Path, req: Parts) -> AppResult> { conduit_compat(move || { let conn = state.db_read()?; let krate: Crate = Crate::by_name(&crate_name).first(&*conn)?; - let mut versions_and_publishers: Vec<(Version, Option)> = krate - .all_versions() - .left_outer_join(users::table) - .select((versions::all_columns, users::all_columns.nullable())) - .load(&*conn)?; + // to keep retrocompatibility, we paginate only if per_page parameter is present + let (mut versions_and_publishers, meta) = if req.query().get("per_page").is_some() { + let pagination_options = PaginationOptions::builder().gather(&req)?; + let data: Paginated<(Version, Option)> = krate + .all_versions() + .left_outer_join(users::table) + .select((versions::all_columns, users::all_columns.nullable())) + .order(versions::created_at.desc()) + .pages_pagination(pagination_options) + .load(&conn)?; + let more = data.next_page_params().is_some(); + + ( + data.into_iter().collect::>(), + Some(json!({ "more": more })), + ) + } else { + let data: Vec<(Version, Option)> = krate + .all_versions() + .left_outer_join(users::table) + .select((versions::all_columns, users::all_columns.nullable())) + .load(&*conn)?; + (data, None) + }; versions_and_publishers .sort_by_cached_key(|(version, _)| Reverse(semver::Version::parse(&version.num).ok())); @@ -344,7 +364,18 @@ pub async fn versions(state: AppState, Path(crate_name): Path) -> AppRes .map(|((v, pb), aas)| EncodableVersion::from(v, &crate_name, pb, aas)) .collect::>(); - Ok(Json(json!({ "versions": versions }))) + // probably here would be simplier with a concrete response type + let response = if let Some(meta) = meta { + json!({ + "versions": versions, + "meta": meta, + }) + } else { + json!({ + "versions": versions, + }) + }; + Ok(Json(response)) }) .await } From 2661840a19ba7b5e4b0d220c225424c9bb947479 Mon Sep 17 00:00:00 2001 From: Marco Napetti Date: Mon, 6 Feb 2023 10:20:44 +0100 Subject: [PATCH 2/2] Cargo fmt --- src/controllers/krate/metadata.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index bfe30674c3f..551cbca30bb 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -8,8 +8,8 @@ use std::cmp::Reverse; use std::str::FromStr; use crate::controllers::frontend_prelude::*; +use crate::controllers::helpers::pagination::{Paginated, PaginationOptions}; use crate::controllers::helpers::Paginate; -use crate::controllers::helpers::pagination::{PaginationOptions, Paginated}; use crate::models::{ Category, Crate, CrateCategory, CrateKeyword, CrateVersions, Keyword, RecentCrateDownloads, @@ -321,7 +321,11 @@ pub async fn readme( /// Handles the `GET /crates/:crate_id/versions` route. // FIXME: Not sure why this is necessary since /crates/:crate_id returns // this information already, but ember is definitely requesting it -pub async fn versions(state: AppState, Path(crate_name): Path, req: Parts) -> AppResult> { +pub async fn versions( + state: AppState, + Path(crate_name): Path, + req: Parts, +) -> AppResult> { conduit_compat(move || { let conn = state.db_read()?; let krate: Crate = Crate::by_name(&crate_name).first(&*conn)?; @@ -336,7 +340,7 @@ pub async fn versions(state: AppState, Path(crate_name): Path, req: Part .pages_pagination(pagination_options) .load(&conn)?; let more = data.next_page_params().is_some(); - + ( data.into_iter().collect::>(), Some(json!({ "more": more })),