show in-progress builds in build queue & build-list pages

migrations/20240624085737_build-status-idx.down.sql

@@ -0,0 +1 @@
+DROP INDEX builds_build_status_idx;

migrations/20240624085737_build-status-idx.up.sql

@@ -0,0 +1 @@
+CREATE INDEX builds_build_status_idx ON builds USING btree (build_status ASC);

src/web/builds.rs

@@ -107,21 +107,24 @@ pub(crate) async fn build_list_json_handler(
             get_builds(&mut conn, &name, &version)
                 .await?
                 .iter()
-                .map(|build| {
+                .filter_map(|build| {
+                    if build.build_status == BuildStatus::InProgress {
+                        return None;
+                    }
                     // for backwards compatibility in this API, we
                     // * convert the build status to a boolean
                     // * already filter out in-progress builds
                     //
                     // even when we start showing in-progress builds in the UI,
                     // we might still not show them here for backwards
                     // compatibility.
-                    serde_json::json!({
+                    Some(serde_json::json!({
                         "id": build.id,
                         "rustc_version": build.rustc_version,
                         "docsrs_version": build.docsrs_version,
                         "build_status": build.build_status.is_success(),
                         "build_time": build.build_time,
-                    })
+                    }))
                 })
                 .collect::<Vec<_>>(),
         ),
@@ -245,8 +248,7 @@ async fn get_builds(
          INNER JOIN crates ON releases.crate_id = crates.id
          WHERE
             crates.name = $1 AND
-            releases.version = $2 AND
-            builds.build_status != 'in_progress'
+            releases.version = $2
          ORDER BY id DESC"#,
         name,
         version.to_string(),
@@ -548,29 +550,25 @@ mod tests {
 
             let response = env.frontend().get("/crate/foo/0.1.0/builds").send()?;
 
-            // FIXME: temporarily we don't show in-progress releases anywhere, which means we don't
-            // show releases without builds anywhere.
-            assert_eq!(response.status(), StatusCode::NOT_FOUND);
-
-            // assert_cache_control(&response, CachePolicy::NoCaching, &env.config());
-            // let page = kuchikiki::parse_html().one(response.text()?);
+            assert_cache_control(&response, CachePolicy::NoCaching, &env.config());
+            let page = kuchikiki::parse_html().one(response.text()?);
 
-            // let rows: Vec<_> = page
-            //     .select("ul > li a.release")
-            //     .unwrap()
-            //     .map(|row| row.text_contents())
-            //     .collect();
+            let rows: Vec<_> = page
+                .select("ul > li a.release")
+                .unwrap()
+                .map(|row| row.text_contents())
+                .collect();
 
-            // assert!(rows.is_empty());
+            assert!(rows.is_empty());
 
-            // let warning = page
-            //     .select_first(".warning")
-            //     .expect("missing warning element")
-            //     .text_contents();
+            let warning = page
+                .select_first(".warning")
+                .expect("missing warning element")
+                .text_contents();
 
-            // assert!(warning.contains("has not built"));
-            // assert!(warning.contains("queued"));
-            // assert!(warning.contains("open an issue"));
+            assert!(warning.contains("has not built"));
+            assert!(warning.contains("queued"));
+            assert!(warning.contains("open an issue"));
 
             Ok(())
         });

src/web/crate_details.rs

@@ -374,8 +374,7 @@ pub(crate) async fn releases_for_crate(
          FROM releases
          INNER JOIN release_build_status ON releases.id = release_build_status.rid
          WHERE
-             releases.crate_id = $1 AND
-             release_build_status.build_status != 'in_progress'"#,
+             releases.crate_id = $1"#,
         crate_id,
     )
     .fetch(&mut *conn)
@@ -1341,25 +1340,22 @@ mod tests {
                 .create()?;
 
             let response = env.frontend().get("/crate/foo/latest").send()?;
-            // FIXME: temporarily we don't show in-progress releases anywhere, which means we don't
-            // show releases without builds anywhere.
-            assert_eq!(response.status(), StatusCode::NOT_FOUND);
-
-            // let page = kuchikiki::parse_html().one(response.text()?);
-            // let link = page
-            //     .select_first("a.pure-menu-link[href='/crate/foo/0.1.0']")
-            //     .unwrap();
-
-            // assert_eq!(
-            //     link.as_node()
-            //         .as_element()
-            //         .unwrap()
-            //         .attributes
-            //         .borrow()
-            //         .get("title")
-            //         .unwrap(),
-            //     "foo-0.1.0 is currently being built"
-            // );
+
+            let page = kuchikiki::parse_html().one(response.text()?);
+            let link = page
+                .select_first("a.pure-menu-link[href='/crate/foo/0.1.0']")
+                .unwrap();
+
+            assert_eq!(
+                link.as_node()
+                    .as_element()
+                    .unwrap()
+                    .attributes
+                    .borrow()
+                    .get("title")
+                    .unwrap(),
+                "foo-0.1.0 is currently being built"
+            );
 
             Ok(())
         });

src/web/mod.rs

@@ -236,6 +236,27 @@ impl MatchedRelease {
     }
 }
 
+fn semver_match<'a, F: Fn(&Release) -> bool>(
+    releases: &'a [Release],
+    req: &VersionReq,
+    filter: F,
+) -> Option<&'a Release> {
+    if let Some(release) = releases
+        .iter()
+        .filter(|release| filter(release))
+        .find(|release| req.matches(&release.version))
+    {
+        Some(release)
+    } else if req == &VersionReq::STAR {
+        // semver `*` does not match pre-releases.
+        // When someone wants the latest release and we have only pre-releases
+        // just return the latest prerelease.
+        return releases.iter().find(|release| filter(release));
+    } else {
+        None
+    }
+}
+
 /// Checks the database for crate releases that match the given name and version.
 ///
 /// `version` may be an exact version number or loose semver version requirement. The return value
@@ -307,14 +328,12 @@ async fn match_version(
         ReqVersion::Semver(version_req) => version_req.clone(),
     };
 
-    // when matching semver requirements, we only want to look at non-yanked releases.
-    let flt = |r: &&Release| r.yanked == Some(false);
-
-    if let Some(release) = releases
-        .iter()
-        .filter(flt)
-        .find(|release| req_semver.matches(&release.version))
-    {
+    // when matching semver requirements,
+    // we generally only want to look at non-yanked releases,
+    // excluding releases which just contain in-progress builds
+    if let Some(release) = semver_match(&releases, &req_semver, |r: &Release| {
+        r.build_status != BuildStatus::InProgress && (r.yanked.is_none() || r.yanked == Some(false))
+    }) {
         return Ok(MatchedRelease {
             name: name.to_owned(),
             corrected_name,
@@ -324,22 +343,17 @@ async fn match_version(
         });
     }
 
-    // semver `*` does not match pre-releases.
-    // When someone wants the latest release and we have only pre-releases
-    // just return the latest prerelease.
-    if req_semver == VersionReq::STAR {
-        return releases
-            .iter()
-            .find(flt)
-            .cloned()
-            .map(|release| MatchedRelease {
-                name: name.to_owned(),
-                corrected_name: corrected_name.clone(),
-                req_version: input_version.clone(),
-                release,
-                all_releases: releases,
-            })
-            .ok_or(AxumNope::VersionNotFound);
+    // when we don't find any match with "normal" releases, we also look into in-progress releases
+    if let Some(release) = semver_match(&releases, &req_semver, |r: &Release| {
+        r.yanked.is_none() || r.yanked == Some(false)
+    }) {
+        return Ok(MatchedRelease {
+            name: name.to_owned(),
+            corrected_name,
+            req_version: input_version.clone(),
+            release: release.clone(),
+            all_releases: releases,
+        });
     }
 
     // Since we return with a CrateNotFound earlier if the db reply is empty,
@@ -1050,6 +1064,35 @@ mod test {
         });
     }
 
+    #[test]
+    fn in_progress_releases_are_ignored_when_others_match() {
+        async_wrapper(|env| async move {
+            let db = env.async_db().await;
+
+            // normal release
+            release("1.0.0", &env).await;
+
+            // in progress release
+            env.async_fake_release()
+                .await
+                .name("foo")
+                .version("1.1.0")
+                .builds(vec![
+                    FakeBuild::default().build_status(BuildStatus::InProgress)
+                ])
+                .create_async()
+                .await?;
+
+            // STAR gives me the prod release
+            assert_eq!(version(Some("*"), db).await, exact("1.0.0"));
+
+            // exact-match query gives me the in progress release
+            assert_eq!(version(Some("=1.1.0"), db).await, exact("1.1.0"));
+
+            Ok(())
+        })
+    }
+
     #[test]
     // https://github.com/rust-lang/docs.rs/issues/1682
     fn prereleases_are_considered_when_others_dont_match() {

src/web/page/templates.rs

@@ -274,7 +274,8 @@ impl IconType {
             IconType::Brand => font_awesome_as_a_crate::Type::Brands,
         };
 
-        let icon_file_string = font_awesome_as_a_crate::svg(type_, icon_name).unwrap_or("");
+        let icon_file_string = font_awesome_as_a_crate::svg(type_, icon_name)
+            .map_err(|err| rinja::Error::Custom(Box::new(err)))?;
 
         let mut classes = vec!["fa-svg"];
         if fw {

src/web/releases.rs

@@ -784,7 +784,8 @@ pub(crate) async fn activity_handler(mut conn: DbConnection) -> AxumResult<impl
 struct BuildQueuePage {
     description: &'static str,
     queue: Vec<QueuedCrate>,
-    active_deployments: Vec<String>,
+    active_cdn_deployments: Vec<String>,
+    in_progress_builds: Vec<(String, String)>,
     csp_nonce: String,
 }
 
@@ -793,8 +794,9 @@ impl_axum_webpage! { BuildQueuePage }
 pub(crate) async fn build_queue_handler(
     Extension(build_queue): Extension<Arc<BuildQueue>>,
     Extension(pool): Extension<Pool>,
+    mut conn: DbConnection,
 ) -> AxumResult<impl IntoResponse> {
-    let (queue, active_deployments) = spawn_blocking(move || {
+    let (queue, active_cdn_deployments) = spawn_blocking(move || {
         let mut queue = build_queue.queued_crates()?;
         for krate in queue.iter_mut() {
             // The priority here is inverted: in the database if a crate has a higher priority it
@@ -820,10 +822,28 @@ pub(crate) async fn build_queue_handler(
     })
     .await?;
 
+    let in_progress_builds: Vec<(String, String)> = sqlx::query!(
+        r#"SELECT
+            crates.name,
+            releases.version
+         FROM builds
+         INNER JOIN releases ON releases.id = builds.rid
+         INNER JOIN crates ON releases.crate_id = crates.id
+         WHERE
+            builds.build_status = 'in_progress'
+         ORDER BY builds.id ASC"#
+    )
+    .fetch_all(&mut *conn)
+    .await?
+    .into_iter()
+    .map(|rec| (rec.name, rec.version))
+    .collect();
+
     Ok(BuildQueuePage {
         description: "crate documentation scheduled to build & deploy",
         queue,
-        active_deployments,
+        active_cdn_deployments,
+        in_progress_builds,
         csp_nonce: String::new(),
     })
 }
@@ -1695,7 +1715,7 @@ mod tests {
 
             let empty = kuchikiki::parse_html().one(web.get("/releases/queue").send()?.text()?);
             assert!(empty
-                .select(".release > strong")
+                .select(".release > div > strong")
                 .expect("missing heading")
                 .any(|el| el.text_contents().contains("active CDN deployments")));