From 8790faabad36222161ac91a47595826a0353d620 Mon Sep 17 00:00:00 2001 From: Wim Looman Date: Tue, 6 Jun 2023 18:00:12 +0200 Subject: [PATCH] Simplify builds.json to return what the clients actually want AFAIK our primary consumers are: * [shields.io](https://github.com/badges/shields/blob/bf1bea8b4a106227cd81a9c5ac88e0266cf9c2c2/services/docsrs/docsrs.service.js#L4-L11) * [crates.io](https://github.com/rust-lang/crates.io/blob/609a7f312429a0e3c99714858b7e7655c8fe36e4/app/models/version.js#L136-L143) They both only care: > will going to `https://docs.rs//` show docs They emulate this by checking whether the latest build is successful, but this relies on docs.rs also using this as the signal (it currently mostly is for other reasons, but we _should_ show docs in cases where a previous build was successful but a newer rebuild failed, and there were previously bugs around proc-macros). So, remove all the extraneous data they don't depend on and just return a simple boolean showing whether the docs are good or not, the same one we use to determine whether to redirect to show a build failure warning or not: https://github.com/rust-lang/docs.rs/blob/f1a7e46a620d763db485a25efb5e510fd3fe0594/src/web/rustdoc.rs#L476 --- src/web/builds.rs | 98 ++++++++++++++--------------------------------- 1 file changed, 29 insertions(+), 69 deletions(-) diff --git a/src/web/builds.rs b/src/web/builds.rs index ced593892..038aac12c 100644 --- a/src/web/builds.rs +++ b/src/web/builds.rs @@ -97,10 +97,18 @@ pub(crate) async fn build_list_json_handler( } }; - let builds = spawn_blocking({ + let rustdoc_status = spawn_blocking({ move || { let mut conn = pool.get()?; - get_builds(&mut conn, &name, &version) + let row = conn.query_one( + "SELECT releases.rustdoc_status + FROM releases + INNER JOIN crates ON releases.crate_id = crates.id + WHERE crates.name = $1 AND releases.version = $2 + ", + &[&name, &version], + )?; + Ok(row.get::<_, bool>("rustdoc_status")) } }) .await?; @@ -108,7 +116,7 @@ pub(crate) async fn build_list_json_handler( Ok(( Extension(CachePolicy::NoStoreMustRevalidate), [(ACCESS_CONTROL_ALLOW_ORIGIN, "*")], - Json(builds), + Json(serde_json::json!([{ "build_status": rustdoc_status }])), ) .into_response()) } @@ -150,7 +158,7 @@ mod tests { test::{assert_cache_control, wrapper, FakeBuild}, web::cache::CachePolicy, }; - use chrono::{DateTime, Duration, Utc}; + use chrono::Duration; use kuchiki::traits::TendrilSink; use reqwest::StatusCode; @@ -197,81 +205,33 @@ mod tests { #[test] fn build_list_json() { + wrapper(|env| { + env.fake_release().name("foo").version("0.1.0").create()?; + + let response = env.frontend().get("/crate/foo/0.1.0/builds.json").send()?; + assert_cache_control(&response, CachePolicy::NoStoreMustRevalidate, &env.config()); + let value: serde_json::Value = serde_json::from_str(&response.text()?)?; + + assert_eq!(value, serde_json::json!([{"build_status": true}])); + + Ok(()) + }); + } + + #[test] + fn build_list_json_failure() { wrapper(|env| { env.fake_release() .name("foo") .version("0.1.0") - .builds(vec![ - FakeBuild::default() - .rustc_version("rustc (blabla 2019-01-01)") - .docsrs_version("docs.rs 1.0.0"), - FakeBuild::default() - .successful(false) - .rustc_version("rustc (blabla 2020-01-01)") - .docsrs_version("docs.rs 2.0.0"), - FakeBuild::default() - .rustc_version("rustc (blabla 2021-01-01)") - .docsrs_version("docs.rs 3.0.0"), - ]) + .build_result_failed() .create()?; let response = env.frontend().get("/crate/foo/0.1.0/builds.json").send()?; assert_cache_control(&response, CachePolicy::NoStoreMustRevalidate, &env.config()); let value: serde_json::Value = serde_json::from_str(&response.text()?)?; - assert_eq!(value.pointer("/0/build_status"), Some(&true.into())); - assert_eq!( - value.pointer("/0/docsrs_version"), - Some(&"docs.rs 3.0.0".into()) - ); - assert_eq!( - value.pointer("/0/rustc_version"), - Some(&"rustc (blabla 2021-01-01)".into()) - ); - assert!(value.pointer("/0/id").unwrap().is_i64()); - assert!(serde_json::from_value::>( - value.pointer("/0/build_time").unwrap().clone() - ) - .is_ok()); - - assert_eq!(value.pointer("/1/build_status"), Some(&false.into())); - assert_eq!( - value.pointer("/1/docsrs_version"), - Some(&"docs.rs 2.0.0".into()) - ); - assert_eq!( - value.pointer("/1/rustc_version"), - Some(&"rustc (blabla 2020-01-01)".into()) - ); - assert!(value.pointer("/1/id").unwrap().is_i64()); - assert!(serde_json::from_value::>( - value.pointer("/1/build_time").unwrap().clone() - ) - .is_ok()); - - assert_eq!(value.pointer("/2/build_status"), Some(&true.into())); - assert_eq!( - value.pointer("/2/docsrs_version"), - Some(&"docs.rs 1.0.0".into()) - ); - assert_eq!( - value.pointer("/2/rustc_version"), - Some(&"rustc (blabla 2019-01-01)".into()) - ); - assert!(value.pointer("/2/id").unwrap().is_i64()); - assert!(serde_json::from_value::>( - value.pointer("/2/build_time").unwrap().clone() - ) - .is_ok()); - - assert!( - value.pointer("/1/build_time").unwrap().as_str().unwrap() - < value.pointer("/0/build_time").unwrap().as_str().unwrap() - ); - assert!( - value.pointer("/2/build_time").unwrap().as_str().unwrap() - < value.pointer("/1/build_time").unwrap().as_str().unwrap() - ); + assert_eq!(value, serde_json::json!([{"build_status": false}])); Ok(()) });