Track when crates are yanked and handle yanked status better

src/docbuilder/queue.rs

@@ -19,12 +19,36 @@ impl DocBuilder {
         // I belive this will fix ordering of queue if we get more than one crate from changes
         changes.reverse();
 
-        for krate in changes.iter().filter(|k| k.kind != ChangeKind::Yanked) {
-            let priority = get_crate_priority(&conn, &krate.name)?;
-            add_crate_to_queue(&conn, &krate.name, &krate.version, priority).ok();
+        for krate in &changes {
+            match krate.kind {
+                ChangeKind::Yanked => {
+                    let res = conn.execute(
+                        "
+                        UPDATE releases
+                            SET yanked = TRUE
+                        FROM crates
+                        WHERE crates.id = releases.crate_id
+                            AND name = $1
+                            AND version = $2
+                        ",
+                        &[&krate.name, &krate.version],
+                    );
+                    match res {
+                        Ok(_) => debug!("{}-{} yanked", krate.name, krate.version),
+                        Err(err) => error!(
+                            "error while setting {}-{} to yanked: {}",
+                            krate.name, krate.version, err
+                        ),
+                    }
+                }
+                ChangeKind::Added => {
+                    let priority = get_crate_priority(&conn, &krate.name)?;
+                    add_crate_to_queue(&conn, &krate.name, &krate.version, priority).ok();
 
-            debug!("{}-{} added into build queue", krate.name, krate.version);
-            add_count += 1;
+                    debug!("{}-{} added into build queue", krate.name, krate.version);
+                    add_count += 1;
+                }
+            }
         }
 
         Ok(add_count)

src/test/fakes.rs

@@ -86,7 +86,7 @@ impl<'a> FakeRelease<'a> {
         self
     }
 
-    pub(crate) fn cratesio_data_yanked(mut self, new: bool) -> Self {
+    pub(crate) fn yanked(mut self, new: bool) -> Self {
         self.cratesio_data.yanked = new;
         self
     }
@@ -139,63 +139,66 @@ impl<'a> FakeRelease<'a> {
         let package = self.package;
         let db = self.db;
 
-        let upload_files = |prefix: &str, files: &[(&str, &[u8])], target: Option<&str>| {
-            let mut path_prefix = tempdir.path().join(prefix);
-            if let Some(target) = target {
-                path_prefix.push(target);
-            }
-            fs::create_dir(&path_prefix)?;
+        let mut source_meta = None;
+        if self.build_result.successful {
+            let upload_files = |prefix: &str, files: &[(&str, &[u8])], target: Option<&str>| {
+                let mut path_prefix = tempdir.path().join(prefix);
+                if let Some(target) = target {
+                    path_prefix.push(target);
+                }
+                fs::create_dir(&path_prefix)?;
+
+                for (path, data) in files {
+                    // allow `src/main.rs`
+                    if let Some(parent) = Path::new(path).parent() {
+                        fs::create_dir_all(path_prefix.join(parent))?;
+                    }
+                    let file = path_prefix.join(&path);
+                    log::debug!("writing file {}", file.display());
+                    fs::write(file, data)?;
+                }
 
-            for (path, data) in files {
-                // allow `src/main.rs`
-                if let Some(parent) = Path::new(path).parent() {
-                    fs::create_dir_all(path_prefix.join(parent))?;
+                let prefix = format!(
+                    "{}/{}/{}/{}",
+                    prefix,
+                    package.name,
+                    package.version,
+                    target.unwrap_or("")
+                );
+                log::debug!("adding directory {} from {}", prefix, path_prefix.display());
+                crate::db::add_path_into_database(&db.conn(), &prefix, path_prefix)
+            };
+
+            let index = [&package.name, "index.html"].join("/");
+            let mut rustdoc_files = self.rustdoc_files;
+            if package.is_library() && !rustdoc_files.iter().any(|(path, _)| path == &index) {
+                rustdoc_files.push((&index, b"default index content"));
+            }
+            for (source_path, data) in &self.source_files {
+                if source_path.starts_with("src/") {
+                    let updated = ["src", &package.name, &source_path[4..]].join("/");
+                    rustdoc_files.push((Box::leak(Box::new(updated)), data));
                 }
-                let file = path_prefix.join(&path);
-                log::debug!("writing file {}", file.display());
-                fs::write(file, data)?;
             }
-
-            let prefix = format!(
-                "{}/{}/{}/{}",
-                prefix,
-                package.name,
-                package.version,
-                target.unwrap_or("")
-            );
-            log::debug!("adding directory {} from {}", prefix, path_prefix.display());
-            crate::db::add_path_into_database(&db.conn(), &prefix, path_prefix)
-        };
-
-        let index = [&package.name, "index.html"].join("/");
-        let mut rustdoc_files = self.rustdoc_files;
-        if package.is_library() && !rustdoc_files.iter().any(|(path, _)| path == &index) {
-            rustdoc_files.push((&index, b"default index content"));
-        }
-        for (source_path, data) in &self.source_files {
-            if source_path.starts_with("src/") {
-                let updated = ["src", &package.name, &source_path[4..]].join("/");
-                rustdoc_files.push((Box::leak(Box::new(updated)), data));
+            let rustdoc_meta = upload_files("rustdoc", &rustdoc_files, None)?;
+            log::debug!("added rustdoc files {}", rustdoc_meta);
+            source_meta = Some(upload_files("source", &self.source_files, None)?);
+            log::debug!("added source files {}", source_meta.as_ref().unwrap());
+
+            for target in &package.targets[1..] {
+                let platform = target.src_path.as_ref().unwrap();
+                upload_files("rustdoc", &rustdoc_files, Some(platform))?;
+                log::debug!("added platform files for {}", platform);
             }
         }
-        let rustdoc_meta = upload_files("rustdoc", &rustdoc_files, None)?;
-        log::debug!("added rustdoc files {}", rustdoc_meta);
-        let source_meta = upload_files("source", &self.source_files, None)?;
-        log::debug!("added source files {}", source_meta);
-
-        for target in &package.targets[1..] {
-            let platform = target.src_path.as_ref().unwrap();
-            upload_files("rustdoc", &rustdoc_files, Some(platform))?;
-            log::debug!("added platform files for {}", platform);
-        }
 
         let release_id = crate::db::add_package_into_database(
             &db.conn(),
             &package,
             tempdir.path(),
             &self.build_result,
             self.default_target.unwrap_or("x86_64-unknown-linux-gnu"),
-            Some(source_meta),
+            source_meta,
             self.doc_targets,
             &self.cratesio_data,
             self.has_docs,

src/web/crate_details.rs

@@ -40,6 +40,7 @@ pub struct CrateDetails {
     github_issues: Option<i32>,
     pub(crate) metadata: MetaData,
     is_library: bool,
+    yanked: bool,
     pub(crate) doc_targets: Vec<String>,
     license: Option<String>,
     documentation_url: Option<String>,
@@ -83,6 +84,7 @@ impl ToJson for CrateDetails {
         m.insert("github_issues".to_string(), self.github_issues.to_json());
         m.insert("metadata".to_string(), self.metadata.to_json());
         m.insert("is_library".to_string(), self.is_library.to_json());
+        m.insert("yanked".to_string(), self.yanked.to_json());
         m.insert("doc_targets".to_string(), self.doc_targets.to_json());
         m.insert("license".to_string(), self.license.to_json());
         m.insert(
@@ -94,17 +96,18 @@ impl ToJson for CrateDetails {
 }
 
 #[derive(Debug, Eq, PartialEq)]
-struct Release {
-    version: String,
-    build_status: bool,
-    yanked: bool,
+pub struct Release {
+    pub version: String,
+    pub build_status: bool,
+    pub yanked: bool,
 }
 
 impl ToJson for Release {
     fn to_json(&self) -> Json {
         let mut m: BTreeMap<String, Json> = BTreeMap::new();
         m.insert("version".to_string(), self.version.to_json());
         m.insert("build_status".to_string(), self.build_status.to_json());
+        m.insert("yanked".to_string(), self.yanked.to_json());
         m.to_json()
     }
 }
@@ -134,6 +137,7 @@ impl CrateDetails {
                 crates.github_forks,
                 crates.github_issues,
                 releases.is_library,
+                releases.yanked,
                 releases.doc_targets,
                 releases.license,
                 releases.documentation_url,
@@ -180,11 +184,11 @@ impl CrateDetails {
             description: rows.get(0).get(4),
             rustdoc_status: rows.get(0).get(11),
             target_name: rows.get(0).get(16),
-            default_target: rows.get(0).get(25),
+            default_target: rows.get(0).get(26),
         };
 
         let doc_targets = {
-            let data: Json = rows.get(0).get(22);
+            let data: Json = rows.get(0).get(23);
             data.as_array()
                 .map(|array| {
                     array
@@ -221,9 +225,10 @@ impl CrateDetails {
             github_issues: rows.get(0).get(20),
             metadata,
             is_library: rows.get(0).get(21),
+            yanked: rows.get(0).get(22),
             doc_targets,
-            license: rows.get(0).get(23),
-            documentation_url: rows.get(0).get(24),
+            license: rows.get(0).get(24),
+            documentation_url: rows.get(0).get(25),
         };
 
         if let Some(repository_url) = crate_details.repository_url.clone() {
@@ -271,10 +276,13 @@ impl CrateDetails {
         Some(crate_details)
     }
 
-    /// Returns the version of the latest release of this crate.
-    pub fn latest_version(&self) -> &str {
-        // releases will always contain at least one element
-        &self.releases[0].version
+    /// Returns the latest non-yanked release of this crate (or latest yanked if they are all
+    /// yanked).
+    pub fn latest_release(&self) -> &Release {
+        self.releases
+            .iter()
+            .find(|release| !release.yanked)
+            .unwrap_or(&self.releases[0])
     }
 }
 
@@ -367,13 +375,13 @@ mod tests {
             db.fake_release()
                 .name("foo")
                 .version("0.0.4")
-                .cratesio_data_yanked(true)
+                .yanked(true)
                 .create()?;
             db.fake_release()
                 .name("foo")
                 .version("0.0.5")
                 .build_result_successful(false)
-                .cratesio_data_yanked(true)
+                .yanked(true)
                 .create()?;
 
             assert_last_successful_build_equals(&db, "foo", "0.0.1", None)?;
@@ -403,7 +411,7 @@ mod tests {
             db.fake_release()
                 .name("foo")
                 .version("0.0.3")
-                .cratesio_data_yanked(true)
+                .yanked(true)
                 .create()?;
 
             assert_last_successful_build_equals(&db, "foo", "0.0.1", None)?;
@@ -427,7 +435,7 @@ mod tests {
             db.fake_release()
                 .name("foo")
                 .version("0.0.3")
-                .cratesio_data_yanked(true)
+                .yanked(true)
                 .create()?;
             db.fake_release().name("foo").version("0.0.4").create()?;
 
@@ -457,7 +465,7 @@ mod tests {
             db.fake_release()
                 .name("foo")
                 .version("0.2.0")
-                .cratesio_data_yanked(true)
+                .yanked(true)
                 .create()?;
             db.fake_release()
                 .name("foo")
@@ -519,14 +527,62 @@ mod tests {
             db.fake_release().name("foo").version("0.0.3").create()?;
             db.fake_release().name("foo").version("0.0.2").create()?;
 
-            let details = CrateDetails::new(&db.conn(), "foo", "0.0.1").unwrap();
-            assert_eq!(details.latest_version(), "0.0.3");
+            for version in &["0.0.1", "0.0.2", "0.0.3"] {
+                let details = CrateDetails::new(&db.conn(), "foo", version).unwrap();
+                assert_eq!(details.latest_release().version, "0.0.3");
+            }
+
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn test_latest_version_ignores_yanked() {
+        crate::test::wrapper(|env| {
+            let db = env.db();
+
+            db.fake_release().name("foo").version("0.0.1").create()?;
+            db.fake_release()
+                .name("foo")
+                .version("0.0.3")
+                .yanked(true)
+                .create()?;
+            db.fake_release().name("foo").version("0.0.2").create()?;
 
-            let details = CrateDetails::new(&db.conn(), "foo", "0.0.2").unwrap();
-            assert_eq!(details.latest_version(), "0.0.3");
+            for version in &["0.0.1", "0.0.2", "0.0.3"] {
+                let details = CrateDetails::new(&db.conn(), "foo", version).unwrap();
+                assert_eq!(details.latest_release().version, "0.0.2");
+            }
 
-            let details = CrateDetails::new(&db.conn(), "foo", "0.0.3").unwrap();
-            assert_eq!(details.latest_version(), "0.0.3");
+            Ok(())
+        })
+    }
+
+    #[test]
+    fn test_latest_version_only_yanked() {
+        crate::test::wrapper(|env| {
+            let db = env.db();
+
+            db.fake_release()
+                .name("foo")
+                .version("0.0.1")
+                .yanked(true)
+                .create()?;
+            db.fake_release()
+                .name("foo")
+                .version("0.0.3")
+                .yanked(true)
+                .create()?;
+            db.fake_release()
+                .name("foo")
+                .version("0.0.2")
+                .yanked(true)
+                .create()?;
+
+            for version in &["0.0.1", "0.0.2", "0.0.3"] {
+                let details = CrateDetails::new(&db.conn(), "foo", version).unwrap();
+                assert_eq!(details.latest_release().version, "0.0.3");
+            }
 
             Ok(())
         })