From 6a5d70c694f80f7918c14b47ce318a644eee021e Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Tue, 31 Dec 2024 16:37:13 +0800 Subject: [PATCH 1/2] controllers/krate/metadata: Sort versions by id This changes the sorting of `crate.versions`. Previously, we sorted versions by semver, which is more computationally expensive compared to sorting by id/date, which can be achieved more easily directly from the database. While this change alters the sorting order, it still provides consistent output and eliminates the need for double semver sorting. --- src/controllers/krate/metadata.rs | 16 ++++------ ...io__tests__routes__crates__read__show.snap | 32 +++++++++---------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 5e4cf1cfa63..2b8ae215722 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -18,7 +18,6 @@ use axum_extra::json; use axum_extra::response::ErasedJson; use diesel::prelude::*; use diesel_async::RunQueryDsl; -use std::cmp::Reverse; use std::str::FromStr; #[derive(Debug, Deserialize, FromRequestParts, utoipa::IntoParams)] @@ -91,15 +90,12 @@ pub async fn find_crate( .ok_or_else(|| crate_not_found(&path.name))?; let mut versions_publishers_and_audit_actions = if include.versions { - let mut versions_and_publishers: Vec<(Version, Option)> = - Version::belonging_to(&krate) - .left_outer_join(users::table) - .select(<(Version, Option)>::as_select()) - .load(&mut conn) - .await?; - - versions_and_publishers - .sort_by_cached_key(|(version, _)| Reverse(semver::Version::parse(&version.num).ok())); + let versions_and_publishers: Vec<(Version, Option)> = Version::belonging_to(&krate) + .left_outer_join(users::table) + .select(<(Version, Option)>::as_select()) + .order_by(versions::id) + .load(&mut conn) + .await?; let versions = versions_and_publishers .iter() diff --git a/src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__read__show.snap b/src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__read__show.snap index 286bb0b1842..8b36379ce9d 100644 --- a/src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__read__show.snap +++ b/src/tests/routes/crates/snapshots/crates_io__tests__routes__crates__read__show.snap @@ -36,8 +36,8 @@ snapshot_kind: text "updated_at": "[datetime]", "versions": [ 1, - 3, - 2 + 2, + 3 ], "yanked": false }, @@ -90,22 +90,22 @@ snapshot_kind: text "crate_size": 0, "created_at": "[datetime]", "description": null, - "dl_path": "/api/v1/crates/foo_show/0.5.1/download", + "dl_path": "/api/v1/crates/foo_show/0.5.0/download", "documentation": null, "downloads": 0, "edition": null, "features": {}, "has_lib": null, "homepage": null, - "id": 3, + "id": 2, "lib_links": null, "license": null, "links": { - "authors": "/api/v1/crates/foo_show/0.5.1/authors", - "dependencies": "/api/v1/crates/foo_show/0.5.1/dependencies", - "version_downloads": "/api/v1/crates/foo_show/0.5.1/downloads" + "authors": "/api/v1/crates/foo_show/0.5.0/authors", + "dependencies": "/api/v1/crates/foo_show/0.5.0/dependencies", + "version_downloads": "/api/v1/crates/foo_show/0.5.0/downloads" }, - "num": "0.5.1", + "num": "0.5.0", "published_by": { "avatar": null, "id": 1, @@ -113,7 +113,7 @@ snapshot_kind: text "name": null, "url": "https://github.com/foo" }, - "readme_path": "/api/v1/crates/foo_show/0.5.1/readme", + "readme_path": "/api/v1/crates/foo_show/0.5.0/readme", "repository": null, "rust_version": null, "updated_at": "[datetime]", @@ -128,22 +128,22 @@ snapshot_kind: text "crate_size": 0, "created_at": "[datetime]", "description": null, - "dl_path": "/api/v1/crates/foo_show/0.5.0/download", + "dl_path": "/api/v1/crates/foo_show/0.5.1/download", "documentation": null, "downloads": 0, "edition": null, "features": {}, "has_lib": null, "homepage": null, - "id": 2, + "id": 3, "lib_links": null, "license": null, "links": { - "authors": "/api/v1/crates/foo_show/0.5.0/authors", - "dependencies": "/api/v1/crates/foo_show/0.5.0/dependencies", - "version_downloads": "/api/v1/crates/foo_show/0.5.0/downloads" + "authors": "/api/v1/crates/foo_show/0.5.1/authors", + "dependencies": "/api/v1/crates/foo_show/0.5.1/dependencies", + "version_downloads": "/api/v1/crates/foo_show/0.5.1/downloads" }, - "num": "0.5.0", + "num": "0.5.1", "published_by": { "avatar": null, "id": 1, @@ -151,7 +151,7 @@ snapshot_kind: text "name": null, "url": "https://github.com/foo" }, - "readme_path": "/api/v1/crates/foo_show/0.5.0/readme", + "readme_path": "/api/v1/crates/foo_show/0.5.1/readme", "repository": null, "rust_version": null, "updated_at": "[datetime]", From 416534cb82eb331af50a2e8e2fc101afa2a33501 Mon Sep 17 00:00:00 2001 From: eth3lbert Date: Tue, 31 Dec 2024 16:50:07 +0800 Subject: [PATCH 2/2] controllers/krate/metadata: Constructing `TopVersions` from fetched versions --- src/controllers/krate/metadata.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs index 2b8ae215722..a6d4b7f1c9b 100644 --- a/src/controllers/krate/metadata.rs +++ b/src/controllers/krate/metadata.rs @@ -7,8 +7,8 @@ use crate::app::AppState; use crate::controllers::krate::CratePath; use crate::models::{ - Category, Crate, CrateCategory, CrateKeyword, Keyword, RecentCrateDownloads, User, Version, - VersionOwnerAction, + Category, Crate, CrateCategory, CrateKeyword, Keyword, RecentCrateDownloads, TopVersions, User, + Version, VersionOwnerAction, }; use crate::schema::*; use crate::util::errors::{bad_request, crate_not_found, AppResult, BoxedAppError}; @@ -160,8 +160,16 @@ pub async fn find_crate( None }; - let top_versions = if include.versions { - Some(krate.top_versions(&mut conn).await?) + let top_versions = if let Some(versions) = versions_publishers_and_audit_actions + .as_ref() + .filter(|_| include.versions) + { + let pairs = versions + .iter() + .filter(|(v, _, _)| !v.yanked) + .cloned() + .map(|(v, _, _)| (v.created_at, v.num)); + Some(TopVersions::from_date_version_pairs(pairs)) } else { None };