Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetch CrateDetails via MatchedRelease to save one SQL query per request #2435

Merged
merged 2 commits into from
Mar 1, 2024
Merged

fetch CrateDetails via MatchedRelease to save one SQL query per request #2435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
63ac89a
fetch CrateDetails via MatchedRelease to save one SQL query per request
syphar Feb 29, 2024
2b99b03
take param by value in CrateDetails::from_matched_release
syphar Mar 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 0 additions & 22 deletions .sqlx/query-908827d37c731be4a6611eacd2e53aa03ae0b38047e0e58c034a3d34f8e29631.json
View file
Open in desktop

This file was deleted.

120 changes: 60 additions & 60 deletions src/web/crate_details.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use crate::{
encode_url_path,
error::{AxumNope, AxumResult},
extractors::{DbConnection, Path},
ReqVersion,
MatchedRelease, ReqVersion,
},
AsyncStorage,
};
Expand All @@ -32,7 +32,7 @@ use std::sync::Arc;
#[derive(Debug, Clone, PartialEq, Serialize)]
pub struct CrateDetails {
name: String,
version: Version,
pub version: Version,
description: Option<String>,
owners: Vec<(String, String)>,
dependencies: Option<Value>,
Expand Down Expand Up @@ -98,11 +98,27 @@ pub struct Release {

impl CrateDetails {
#[tracing::instrument(skip(conn))]
pub async fn new(
pub(crate) async fn from_matched_release(
conn: &mut sqlx::PgConnection,
release: MatchedRelease,
) -> Result<Self> {
Ok(Self::new(
conn,
&release.name,
&release.release.version,
Some(release.req_version),
release.all_releases,
)
.await?
.unwrap())
}

async fn new(
conn: &mut sqlx::PgConnection,
name: &str,
version: &Version,
req_version: Option<ReqVersion>,
prefetched_releases: Vec<Release>,
) -> Result<Option<CrateDetails>, anyhow::Error> {
let krate = match sqlx::query!(
r#"SELECT
Expand Down Expand Up @@ -163,9 +179,6 @@ impl CrateDetails {
None => return Ok(None),
};

// get releases, sorted by semver
let releases = releases_for_crate(conn, krate.crate_id).await?;

let repository_metadata = krate.repo_host.map(|_| RepositoryMetadata {
issues: krate.repo_issues.unwrap(),
stars: krate.repo_stars.unwrap(),
Expand Down Expand Up @@ -205,7 +218,7 @@ impl CrateDetails {
keywords: krate.keywords,
have_examples: krate.have_examples,
target_name: krate.target_name,
releases,
releases: prefetched_releases,
repository_metadata,
metadata,
is_library: krate.is_library,
Expand Down Expand Up @@ -399,21 +412,17 @@ pub(crate) async fn crate_details_handler(
)
})?;

let version = match_version(&mut conn, &params.name, &req_version)
let matched_release = match_version(&mut conn, &params.name, &req_version)
.await?
.assume_exact_name()?
.into_canonical_req_version_or_else(|version| {
AxumNope::Redirect(
format!("/crate/{}/{}", &params.name, version),
CachePolicy::ForeverInCdn,
)
})?
.into_version();
})?;

let mut details =
CrateDetails::new(&mut conn, &params.name, &version, Some(req_version.clone()))
.await?
.ok_or(AxumNope::VersionNotFound)?;
let mut details = CrateDetails::from_matched_release(&mut conn, matched_release).await?;

match details.fetch_readme(&storage).await {
Ok(readme) => details.readme = readme.or(details.readme),
Expand Down Expand Up @@ -670,23 +679,44 @@ mod tests {
assert_cache_control, assert_redirect, assert_redirect_cached, async_wrapper, wrapper,
TestDatabase, TestEnvironment,
};
use anyhow::{Context, Error};
use anyhow::Error;
use kuchikiki::traits::TendrilSink;
use semver::Version;
use std::collections::HashMap;

async fn crate_details(
conn: &mut sqlx::PgConnection,
name: &str,
version: &str,
req_version: Option<ReqVersion>,
) -> CrateDetails {
let crate_id: i32 = sqlx::query_scalar!("SELECT id FROM crates WHERE name = $1", name)
.fetch_one(&mut *conn)
.await
.unwrap();

let releases = releases_for_crate(&mut *conn, crate_id).await.unwrap();

CrateDetails::new(
&mut *conn,
name,
&Version::parse(version).unwrap(),
req_version,
releases,
)
.await
.unwrap()
.unwrap()
}

async fn assert_last_successful_build_equals(
db: &TestDatabase,
package: &str,
version: &str,
expected_last_successful_build: Option<&str>,
) -> Result<(), Error> {
let mut conn = db.async_conn().await;
let details =
CrateDetails::new(&mut conn, package, &Version::parse(version).unwrap(), None)
.await
.with_context(|| anyhow::anyhow!("could not fetch crate details"))?
.unwrap();
let details = crate_details(&mut conn, package, version, None).await;

assert_eq!(
details.last_successful_build,
Expand Down Expand Up @@ -851,10 +881,7 @@ mod tests {

let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &"0.2.0".parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", "0.2.0", None).await
});

assert_eq!(
Expand Down Expand Up @@ -972,10 +999,7 @@ mod tests {
for version in &["0.0.1", "0.0.2", "0.0.3"] {
let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", version, None).await
});
assert_eq!(
details.latest_release().unwrap().version,
Expand All @@ -1002,10 +1026,7 @@ mod tests {
for version in &["0.0.1", "0.0.2", "0.0.3-pre.1"] {
let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", version, None).await
});
assert_eq!(
details.latest_release().unwrap().version,
Expand Down Expand Up @@ -1033,10 +1054,7 @@ mod tests {
for version in &["0.0.1", "0.0.2", "0.0.3"] {
let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &(version.parse().unwrap()), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", version, None).await
});
assert_eq!(
details.latest_release().unwrap().version,
Expand Down Expand Up @@ -1072,10 +1090,7 @@ mod tests {
for version in &["0.0.1", "0.0.2", "0.0.3"] {
let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &version.parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", version, None).await
});
assert_eq!(
details.latest_release().unwrap().version,
Expand Down Expand Up @@ -1132,10 +1147,7 @@ mod tests {

let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", "0.0.1", None).await
});
assert_eq!(
details.owners,
Expand All @@ -1158,10 +1170,7 @@ mod tests {

let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", "0.0.1", None).await
});
let mut owners = details.owners;
owners.sort();
Expand All @@ -1185,10 +1194,7 @@ mod tests {

let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", "0.0.1", None).await
});
assert_eq!(
details.owners,
Expand All @@ -1207,10 +1213,7 @@ mod tests {

let details = env.runtime().block_on(async move {
let mut conn = db.async_conn().await;
CrateDetails::new(&mut conn, "foo", &"0.0.1".parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "foo", "0.0.1", None).await
});
assert_eq!(
details.owners,
Expand Down Expand Up @@ -1654,10 +1657,7 @@ mod tests {

let details = env.runtime().block_on(async move {
let mut conn = env.async_db().await.async_conn().await;
CrateDetails::new(&mut conn, "dummy", &"0.5.0".parse().unwrap(), None)
.await
.unwrap()
.unwrap()
crate_details(&mut conn, "dummy", "0.5.0", None).await
});
assert!(matches!(
env.runtime()
Expand Down
11 changes: 9 additions & 2 deletions src/web/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl FromStr for ReqVersion {
}

#[derive(Debug)]
struct MatchedRelease {
pub(crate) struct MatchedRelease {
/// crate name
pub name: String,

Expand All @@ -125,6 +125,9 @@ struct MatchedRelease {

/// the matched release
pub release: crate_details::Release,

/// all releases since we have them anyways and so we can pass them to CrateDetails
all_releases: Vec<crate_details::Release>,
}

impl MatchedRelease {
Expand Down Expand Up @@ -274,6 +277,7 @@ async fn match_version(
corrected_name,
req_version: input_version.clone(),
release: release.clone(),
all_releases: releases,
});
}

Expand Down Expand Up @@ -302,6 +306,7 @@ async fn match_version(
corrected_name,
req_version: input_version.clone(),
release: release.clone(),
all_releases: releases,
});
}

Expand All @@ -311,11 +316,13 @@ async fn match_version(
if req_semver == VersionReq::STAR {
return releases
.first()
.cloned()
.map(|release| MatchedRelease {
name: name.to_owned(),
corrected_name: corrected_name.clone(),
req_version: input_version.clone(),
release: release.clone(),
release,
all_releases: releases,
})
.ok_or(AxumNope::VersionNotFound);
}
Expand Down
Loading
Loading