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.

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

Only put platforms list is less than 10, otherwise load with AJAX #2181

Merged
merged 14 commits into from
Sep 7, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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
2 changes: 1 addition & 1 deletion src/docbuilder/limits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ impl Default for Limits {
Self {
memory: 3 * 1024 * 1024 * 1024, // 3 GB
timeout: Duration::from_secs(15 * 60), // 15 minutes
targets: 10,
targets: crate::DEFAULT_MAX_TARGETS,
networking: false,
max_log_size: 100 * 1024, // 100 KB
}
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,6 @@ pub const BUILD_VERSION: &str = concat!(
/// Example:
/// `s3://rust-docs-rs//rustdoc-static/something.css`
pub const RUSTDOC_STATIC_STORAGE_PREFIX: &str = "/rustdoc-static/";

/// Maximum number of targets allowed for a crate to be documented on.
pub const DEFAULT_MAX_TARGETS: usize = 10;
207 changes: 205 additions & 2 deletions src/web/crate_details.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use super::{markdown, match_version, MatchSemver, MetaData};
use crate::utils::{get_correct_docsrs_style_file, report_error, spawn_blocking};
use crate::web::rustdoc::RustdocHtmlParams;
use crate::web::{axum_cached_redirect, match_version_axum};
use crate::{
db::Pool,
impl_axum_webpage,
Expand All @@ -15,6 +17,7 @@ use crate::{
use anyhow::{Context, Result};
use axum::{
extract::{Extension, Path},
http::Uri,
response::{IntoResponse, Response as AxumResponse},
};
use chrono::{DateTime, Utc};
Expand All @@ -23,6 +26,7 @@ use serde::Deserialize;
use serde::{ser::Serializer, Serialize};
use serde_json::Value;
use std::sync::Arc;
use tracing::{instrument, trace};

// TODO: Add target name and versions

Expand Down Expand Up @@ -466,6 +470,160 @@ pub(crate) async fn get_all_releases(
Ok(res.into_response())
}

#[derive(Debug, Clone, PartialEq, Serialize)]
struct ShortMetadata {
name: String,
version_or_latest: String,
doc_targets: Vec<String>,
}

#[derive(Debug, Clone, PartialEq, Serialize)]
struct PlatformList {
metadata: ShortMetadata,
inner_path: String,
use_direct_platform_links: bool,
current_target: String,
}

impl_axum_webpage! {
PlatformList = "rustdoc/platforms.html",
cpu_intensive_rendering = true,
}

#[tracing::instrument]
pub(crate) async fn get_all_platforms(
Path(params): Path<RustdocHtmlParams>,
Extension(pool): Extension<Pool>,
uri: Uri,
) -> AxumResult<AxumResponse> {
let req_path: String = params.path.unwrap_or_default();
let req_path: Vec<&str> = req_path.split('/').collect();

let release_found = match_version_axum(&pool, &params.name, Some(&params.version)).await?;
trace!(?release_found, "found release");

// Convenience function to allow for easy redirection
#[instrument]
fn redirect(
name: &str,
vers: &str,
path: &[&str],
cache_policy: CachePolicy,
) -> AxumResult<AxumResponse> {
trace!("redirect");
// Format and parse the redirect url
Ok(axum_cached_redirect(
encode_url_path(&format!("/platforms/{}/{}/{}", name, vers, path.join("/"))),
cache_policy,
)?
.into_response())
}

let (version, version_or_latest) = match release_found.version {
MatchSemver::Exact((version, _)) => {
// Redirect when the requested crate name isn't correct
if let Some(name) = release_found.corrected_name {
return redirect(&name, &version, &req_path, CachePolicy::NoCaching);
}

(version.clone(), version)
}

MatchSemver::Latest((version, _)) => {
// Redirect when the requested crate name isn't correct
if let Some(name) = release_found.corrected_name {
return redirect(&name, "latest", &req_path, CachePolicy::NoCaching);
}

(version, "latest".to_string())
}

// Redirect when the requested version isn't correct
MatchSemver::Semver((v, _)) => {
// to prevent cloudfront caching the wrong artifacts on URLs with loose semver
// versions, redirect the browser to the returned version instead of loading it
// immediately
return redirect(&params.name, &v, &req_path, CachePolicy::ForeverInCdn);
Nemo157 marked this conversation as resolved.
Show resolved Hide resolved
}
};

let (name, doc_targets, releases, default_target): (String, Vec<String>, Vec<Release>, String) =
spawn_blocking({
let pool = pool.clone();
move || {
let mut conn = pool.get()?;
let query = "
SELECT
crates.id,
crates.name,
releases.default_target,
releases.doc_targets
FROM releases
INNER JOIN crates ON releases.crate_id = crates.id
WHERE crates.name = $1 AND releases.version = $2;";

let rows = conn.query(query, &[&params.name, &version])?;

let krate = if rows.is_empty() {
return Err(AxumNope::CrateNotFound.into());
} else {
&rows[0]
};

// get releases, sorted by semver
let releases = releases_for_crate(&mut *conn, krate.get("id"))?;

Ok((
krate.get("name"),
MetaData::parse_doc_targets(krate.get("doc_targets")),
releases,
krate.get("default_target"),
))
}
})
.await?;

let latest_release = releases
.iter()
.find(|release| release.version.pre.is_empty() && !release.yanked)
.unwrap_or(&releases[0]);

// The path within this crate version's rustdoc output
let (target, inner_path) = {
let mut inner_path = req_path.clone();

let target = if inner_path.len() > 1 && doc_targets.iter().any(|s| s == inner_path[0]) {
inner_path.remove(0)
} else {
""
};

(target, inner_path.join("/"))
};

let current_target = if latest_release.build_status {
if target.is_empty() {
default_target
} else {
target.to_owned()
}
} else {
String::new()
};

let res = PlatformList {
metadata: ShortMetadata {
name,
version_or_latest: version_or_latest.to_string(),
doc_targets,
},
inner_path,
use_direct_platform_links: true,
GuillaumeGomez marked this conversation as resolved.
Show resolved Hide resolved
current_target,
};
Ok(res.into_response())
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down Expand Up @@ -1087,12 +1245,15 @@ mod tests {
.add_target("x86_64-pc-windows-msvc")
.create()?;

let response = env.frontend().get("/crate/dummy/0.4.0").send()?;
let response = env
.frontend()
.get("/menus/platforms/dummy/0.4.0/x86_64-pc-windows-msvc")
.send()?;
assert!(response.status().is_success());

let platform_links: Vec<(String, String)> = kuchikiki::parse_html()
.one(response.text()?)
.select(r#"a[aria-label="Platform"] + ul li a"#)
.select(r#"li a"#)
.expect("invalid selector")
.map(|el| {
let attributes = el.attributes.borrow();
Expand All @@ -1113,6 +1274,48 @@ mod tests {
});
}

// Ensure that if there are more than a given number of targets, it will not generate them in
// the HTML directly (they will be loaded by AJAX if the user opens the menu).
#[test]
#[allow(clippy::assertions_on_constants)]
fn platform_menu_ajax() {
assert!(crate::DEFAULT_MAX_TARGETS > 2);

fn check_count(nb_targets: usize, expected: usize) {
wrapper(|env| {
let mut rel = env
.fake_release()
.name("dummy")
.version("0.4.0")
.rustdoc_file("dummy/index.html")
.rustdoc_file("x86_64-pc-windows-msvc/dummy/index.html")
.default_target("x86_64-unknown-linux-gnu");

for nb in 0..nb_targets - 1 {
rel = rel.add_target(&format!("x86_64-pc-windows-msvc{nb}"));
}
rel.create()?;

let response = env.frontend().get("/crate/dummy/0.4.0").send()?;
assert!(response.status().is_success());

let nb_li = kuchikiki::parse_html()
.one(response.text()?)
.select(r#"#platforms li a"#)
.expect("invalid selector")
.count();
assert_eq!(nb_li, expected);
Ok(())
});
}

// First we check that with 2 releases, the platforms list should be in the HTML.
check_count(2, 2);
// Then we check the same thing but with number of targets equal
// to `DEFAULT_MAX_TARGETS`.
check_count(crate::DEFAULT_MAX_TARGETS, 0);
}

#[test]
fn latest_url() {
wrapper(|env| {
Expand Down
8 changes: 6 additions & 2 deletions src/web/page/web_page.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,12 @@ macro_rules! impl_axum_webpage {


response.extensions_mut().insert($crate::web::page::web_page::DelayedTemplateRender {
context: ::tera::Context::from_serialize(&self)
.expect("could not create tera context from web-page"),
context: {
let mut c = ::tera::Context::from_serialize(&self)
.expect("could not create tera context from web-page");
c.insert("DEFAULT_MAX_TARGETS", &$crate::DEFAULT_MAX_TARGETS);
c
},
template: {
let template: fn(&Self) -> ::std::borrow::Cow<'static, str> = $template;
template(&self).to_string()
Expand Down
20 changes: 16 additions & 4 deletions src/web/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,10 +180,6 @@ pub(super) fn build_axum_routes() -> AxumRouter {
"/crate/:name",
get_internal(super::crate_details::crate_details_handler),
)
.route(
"/:name/releases",
get_internal(super::crate_details::get_all_releases),
)
.route_with_tsr(
"/crate/:name/:version",
get_internal(super::crate_details::crate_details_handler),
Expand Down Expand Up @@ -240,6 +236,22 @@ pub(super) fn build_axum_routes() -> AxumRouter {
"/crate/:name/:version/source/*path",
get_internal(super::source::source_browser_handler),
)
.route(
"/menus/platforms/:name/:version/:target",
get_internal(super::crate_details::get_all_platforms),
)
.route(
"/menus/platforms/:name/:version/:target/",
get_internal(super::crate_details::get_all_platforms),
)
.route(
"/menus/platforms/:name/:version/:target/*path",
get_internal(super::crate_details::get_all_platforms),
)
.route(
"/menus/releases/:name",
get_internal(super::crate_details::get_all_releases),
)
.route(
"/-/rustdoc.static/*path",
get_internal(super::rustdoc::static_asset_handler),
Expand Down
21 changes: 13 additions & 8 deletions src/web/rustdoc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ impl RustdocPage {
let is_latest_url = self.is_latest_url;

// Build the page of documentation
let ctx = tera::Context::from_serialize(self).context("error creating tera context")?;
let mut ctx = tera::Context::from_serialize(self).context("error creating tera context")?;
ctx.insert("DEFAULT_MAX_TARGETS", &crate::DEFAULT_MAX_TARGETS);

// Extract the head and body of the rustdoc file so that we can insert it into our own html
// while logging OOM errors from html rewriting
Expand Down Expand Up @@ -349,17 +350,17 @@ impl RustdocPage {
}
}

#[derive(Clone, Deserialize)]
#[derive(Clone, Deserialize, Debug)]
pub(crate) struct RustdocHtmlParams {
name: String,
version: String,
pub(crate) name: String,
pub(crate) version: String,
// both target and path are only used for matching the route.
// The actual path is read from the request `Uri` because
// we have some static filenames directly in the routes.
#[allow(dead_code)]
target: Option<String>,
pub(crate) target: Option<String>,
#[allow(dead_code)]
path: Option<String>,
pub(crate) path: Option<String>,
}

/// Serves documentation generated by rustdoc.
Expand Down Expand Up @@ -2258,8 +2259,12 @@ mod test {
.create()?;

// test rustdoc pages stay on the documentation
let page = kuchikiki::parse_html()
.one(env.frontend().get("/hexponent/releases").send()?.text()?);
let page = kuchikiki::parse_html().one(
env.frontend()
.get("/menus/releases/hexponent")
.send()?
.text()?,
);
let selector =
r#"ul > li a[href="/crate/hexponent/0.3.1/target-redirect/hexponent/index.html"]"#
.to_string();
Expand Down
Loading