From c2d42965193db4ba31796aa90de553700a49e40b Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sun, 7 Nov 2021 14:51:29 +0100 Subject: [PATCH 1/4] change latest version ID on crate to match CrateDetails.latest_version --- src/db/add_package.rs | 22 +++--- src/db/migrate.rs | 141 +++++++++++++++++++++++----------- src/test/mod.rs | 12 +-- src/web/crate_details.rs | 160 +++++++++++++++++---------------------- src/web/mod.rs | 2 +- src/web/rustdoc.rs | 13 +++- 6 files changed, 190 insertions(+), 160 deletions(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index 0af5fbb44..d57910133 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -1,10 +1,3 @@ -use std::{ - collections::{HashMap, HashSet}, - fs, - io::{BufRead, BufReader}, - path::Path, -}; - use crate::{ db::types::Feature, docbuilder::{BuildResult, DocCoverage}, @@ -12,11 +5,19 @@ use crate::{ index::api::{CrateData, CrateOwner, ReleaseData}, storage::CompressionAlgorithm, utils::MetadataPackage, + web::crate_details::CrateDetails, }; +use anyhow::{anyhow, Context}; use log::{debug, info}; use postgres::Client; use serde_json::Value; use slug::slugify; +use std::{ + collections::{HashMap, HashSet}, + fs, + io::{BufRead, BufReader}, + path::Path, +}; /// Adds a package into database. /// @@ -127,12 +128,15 @@ pub(crate) fn add_package_into_database( add_keywords_into_database(conn, metadata_pkg, release_id)?; add_compression_into_database(conn, compression_algorithms.into_iter(), release_id)?; - // Update the crates table with the new release + let crate_details = CrateDetails::new(conn, &metadata_pkg.name, &metadata_pkg.version, None) + .with_context(|| anyhow!("error when fetching crate-details"))? + .ok_or_else(|| anyhow!("crate details not found directly after creating them"))?; + conn.execute( "UPDATE crates SET latest_version_id = $2 WHERE id = $1", - &[&crate_id, &release_id], + &[&crate_id, &crate_details.latest_release().id], )?; Ok(release_id) diff --git a/src/db/migrate.rs b/src/db/migrate.rs index 053d68063..0b71af9d0 100644 --- a/src/db/migrate.rs +++ b/src/db/migrate.rs @@ -5,19 +5,8 @@ use postgres::{Client, Error as PostgresError, Transaction}; use schemamama::{Migration, Migrator, Version}; use schemamama_postgres::{PostgresAdapter, PostgresMigration}; -/// Creates a new PostgresMigration from upgrade and downgrade queries. -/// Downgrade query should return database to previous state. -/// -/// Example: -/// -/// ``` -/// let my_migration = migration!(100, -/// "Create test table", -/// "CREATE TABLE test ( id SERIAL);", -/// "DROP TABLE test;"); -/// ``` macro_rules! migration { - ($context:expr, $version:expr, $description:expr, $up:expr, $down:expr $(,)?) => {{ + ($context:expr, $version:expr, $description:expr, $up_func:expr, $down_func:expr $(,)?) => {{ struct Amigration; impl Migration for Amigration { fn version(&self) -> Version { @@ -41,7 +30,8 @@ macro_rules! migration { self.version(), self.description() ); - transaction.batch_execute($up).map(|_| ()) + + $up_func(transaction) } fn down(&self, transaction: &mut Transaction) -> Result<(), PostgresError> { let level = if cfg!(test) { @@ -55,13 +45,29 @@ macro_rules! migration { self.version(), self.description() ); - transaction.batch_execute($down).map(|_| ()) + $down_func(transaction) } } Box::new(Amigration) }}; } +macro_rules! sql_migration { + ($context:expr, $version:expr, $description:expr, $up:expr, $down:expr $(,)?) => {{ + migration!( + $context, + $version, + $description, + |transaction: &mut Transaction| -> Result<(), PostgresError> { + transaction.batch_execute($up).map(|_| ()) + }, + |transaction: &mut Transaction| -> Result<(), PostgresError> { + transaction.batch_execute($down).map(|_| ()) + } + ) + }}; +} + pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Result<()> { conn.execute( "CREATE TABLE IF NOT EXISTS database_versions (version BIGINT PRIMARY KEY);", @@ -73,7 +79,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res let mut migrator = Migrator::new(adapter); let migrations: Vec> = vec![ - migration!( + sql_migration!( context, // version 1, @@ -190,7 +196,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res "DROP TABLE authors, author_rels, keyword_rels, keywords, owner_rels, owners, releases, crates, builds, queue, files, config;" ), - migration!( + sql_migration!( context, // version 2, @@ -201,7 +207,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "ALTER TABLE queue DROP COLUMN priority;" ), - migration!( + sql_migration!( context, // version 3, @@ -216,7 +222,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "DROP TABLE sandbox_overrides;" ), - migration!( + sql_migration!( context, 4, "Make more fields not null", @@ -227,7 +233,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ALTER COLUMN yanked DROP NOT NULL, ALTER COLUMN downloads DROP NOT NULL" ), - migration!( + sql_migration!( context, // version 5, @@ -238,7 +244,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "ALTER TABLE releases ALTER COLUMN target_name DROP NOT NULL", ), - migration!( + sql_migration!( context, // version 6, @@ -251,7 +257,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "DROP TABLE blacklisted_crates;" ), - migration!( + sql_migration!( context, // version 7, @@ -262,7 +268,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "ALTER TABLE sandbox_overrides ALTER COLUMN max_memory_bytes TYPE INTEGER;" ), - migration!( + sql_migration!( context, // version 8, @@ -275,7 +281,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res "ALTER TABLE releases ALTER COLUMN default_target DROP NOT NULL; ALTER TABLE releases ALTER COLUMN default_target DROP DEFAULT", ), - migration!( + sql_migration!( context, // version 9, @@ -286,7 +292,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "ALTER TABLE sandbox_overrides DROP COLUMN max_targets;" ), - migration!( + sql_migration!( context, // version 10, @@ -309,7 +315,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res DROP FUNCTION normalize_crate_name; " ), - migration!( + sql_migration!( context, // version 11, @@ -323,7 +329,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "DROP TABLE crate_priorities;", ), - migration!( + sql_migration!( context, // version 12, @@ -338,7 +344,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ALTER TABLE releases ALTER COLUMN doc_targets DROP NOT NULL; " ), - migration!( + sql_migration!( context, // version 13, @@ -353,7 +359,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ADD COLUMN content tsvector, ADD COLUMN versions JSON DEFAULT '[]';" ), - migration!( + sql_migration!( context, // version 14, @@ -377,7 +383,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res "DROP TABLE compression_rels; ALTER TABLE files DROP COLUMN compression;" ), - migration!( + sql_migration!( context, // version 15, @@ -393,7 +399,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res -- Nope, this is a pure database fix, no going back. " ), - migration!( + sql_migration!( context, // version 16, @@ -410,7 +416,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res // downgrade query "DROP TABLE doc_coverage;" ), - migration!( + sql_migration!( context, // version 17, @@ -451,7 +457,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ALTER TABLE crates ALTER COLUMN github_stars DROP NOT NULL; " ), - migration!( + sql_migration!( context, // version 18, @@ -470,7 +476,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res DROP COLUMN items_with_examples; " ), - migration!( + sql_migration!( context, // version 19, @@ -487,7 +493,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res DROP TYPE feature; " ), - migration!( + sql_migration!( context, 20, // description @@ -501,7 +507,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ALTER TABLE queue DROP COLUMN registry; " ), - migration!( + sql_migration!( context, 21, // description @@ -515,7 +521,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ALTER TYPE feature DROP ATTRIBUTE optional_dependency; " ), - migration!( + sql_migration!( context, 22, // description @@ -542,7 +548,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res DROP TABLE github_repos; " ), - migration!( + sql_migration!( context, 23, // description @@ -568,7 +574,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ADD COLUMN github_last_update TIMESTAMP; " ), - migration!( + sql_migration!( context, 24, "drop unused `date_added` columns", @@ -579,7 +585,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res "ALTER TABLE queue ADD COLUMN date_added TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP; ALTER TABLE files ADD COLUMN date_added TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP;", ), - migration!( + sql_migration!( context, 25, "migrate timestamp to be timezone aware", @@ -614,7 +620,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ALTER release_time TYPE timestamp USING release_time AT TIME ZONE 'UTC'; ", ), - migration!( + sql_migration!( context, 26, "create indexes for crates, github_repos and releases", @@ -631,7 +637,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res DROP INDEX github_repos_stars_idx; ", ), - migration!( + sql_migration!( context, 27, "delete the authors and author_rels", @@ -656,7 +662,7 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res ALTER TABLE releases ADD COLUMN authors JSON; ", ), - migration!( + sql_migration!( context, 28, // description @@ -744,21 +750,66 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res DROP TABLE repositories; ", ), - migration!( + sql_migration!( context, 29, "Rename cratesfyi_version to docsrs_version", "ALTER TABLE builds RENAME COLUMN cratesfyi_version TO docsrs_version", "ALTER TABLE builds RENAME COLUMN docsrs_version TO cratesfyi_version", ), - migration!( + sql_migration!( context, 30, "add archive-storage marker for releases", "ALTER TABLE releases ADD COLUMN archive_storage BOOL NOT NULL DEFAULT FALSE;", "ALTER TABLE releases DROP COLUMN archive_storage;", ), - migration!( + sql_migration!( context, 31, "add index on builds.build_time", "CREATE INDEX builds_build_time_idx ON builds (build_time DESC);", "DROP INDEX builds_build_time_idx;", ), + migration!( + context, + 32, + "update latest_version_id on all crates", + |transaction: &mut Transaction| -> Result<(), PostgresError> { + use crate::web::crate_details::CrateDetails; + let rows = transaction.query( + "SELECT crates.name, max(releases.version) as max_version_id + FROM crates + INNER JOIN releases ON crates.id = releases.crate_id + GROUP BY crates.name", + &[], + )?; + + let update_version_query = transaction.prepare( + "UPDATE crates + SET latest_version_id = $2 + WHERE id = $1", + )?; + + for row in rows.into_iter() { + if let Some(details) = CrateDetails::new(transaction, row.get(0), row.get(1), None)? { + transaction.execute( + &update_version_query, + &[&details.crate_id, &details.latest_release().id], + )?; + } + } + + Ok(()) + }, + |transaction: &mut Transaction| -> Result<(), PostgresError> { + transaction + .execute( + "UPDATE crates + SET latest_version_id = ( + SELECT max(id) + FROM releases + WHERE releases.crate_id = crates.id + )", + &[], + ) + .map(|_| ()) + } + ) ]; for migration in migrations { diff --git a/src/test/mod.rs b/src/test/mod.rs index d4b5badc1..f662f9663 100644 --- a/src/test/mod.rs +++ b/src/test/mod.rs @@ -295,7 +295,6 @@ impl Context for TestEnvironment { pub(crate) struct TestDatabase { pool: Pool, schema: String, - repository_stats_updater: RepositoryStatsUpdater, } impl TestDatabase { @@ -305,7 +304,6 @@ impl TestDatabase { let schema = format!("docs_rs_test_schema_{}", rand::random::()); let pool = Pool::new_with_schema(config, metrics, &schema)?; - let repository_stats_updater = RepositoryStatsUpdater::new(config, pool.clone()); let mut conn = Connection::connect(&config.database_url, postgres::NoTls)?; conn.batch_execute(&format!( @@ -340,11 +338,7 @@ impl TestDatabase { .collect(); conn.batch_execute(&query)?; - Ok(TestDatabase { - pool, - schema, - repository_stats_updater, - }) + Ok(TestDatabase { pool, schema }) } pub(crate) fn pool(&self) -> Pool { @@ -356,10 +350,6 @@ impl TestDatabase { .get() .expect("failed to get a connection out of the pool") } - - pub(crate) fn repository_stats_updater(&self) -> &RepositoryStatsUpdater { - &self.repository_stats_updater - } } impl Drop for TestDatabase { diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index 5f07746df..b8c66d8e7 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -3,7 +3,7 @@ use crate::{db::Pool, impl_webpage, repositories::RepositoryStatsUpdater, web::p use chrono::{DateTime, Utc}; use iron::prelude::*; use iron::Url; -use postgres::Client; +use postgres::GenericClient; use router::Router; use serde::{ser::Serializer, Serialize}; use serde_json::Value; @@ -68,6 +68,7 @@ where #[derive(Debug, Clone, Eq, PartialEq, Serialize)] pub struct Release { + pub id: i32, pub version: semver::Version, pub build_status: bool, pub yanked: bool, @@ -76,11 +77,11 @@ pub struct Release { impl CrateDetails { pub fn new( - conn: &mut Client, + conn: &mut impl GenericClient, name: &str, version: &str, - up: &RepositoryStatsUpdater, - ) -> Option { + up: Option<&RepositoryStatsUpdater>, + ) -> Result, postgres::Error> { // get all stuff, I love you rustfmt let query = " SELECT @@ -122,10 +123,10 @@ impl CrateDetails { LEFT JOIN repositories ON releases.repository_id = repositories.id WHERE crates.name = $1 AND releases.version = $2;"; - let rows = conn.query(query, &[&name, &version]).unwrap(); + let rows = conn.query(query, &[&name, &version])?; let krate = if rows.is_empty() { - return None; + return Ok(None); } else { &rows[0] }; @@ -134,7 +135,7 @@ impl CrateDetails { let release_id: i32 = krate.get("release_id"); // get releases, sorted by semver - let releases = releases_for_crate(conn, crate_id); + let releases = releases_for_crate(conn, crate_id)?; let repository_metadata = krate @@ -144,7 +145,7 @@ impl CrateDetails { stars: krate.get("repo_stars"), forks: krate.get("repo_forks"), name: krate.get("repo_name"), - icon: up.get_icon_name(&host), + icon: up.map_or("code-branch", |u| u.get_icon_name(&host)), }); let metadata = MetaData { @@ -196,15 +197,13 @@ impl CrateDetails { }; // get owners - let owners = conn - .query( - "SELECT login, avatar + let owners = conn.query( + "SELECT login, avatar FROM owners INNER JOIN owner_rels ON owner_rels.oid = owners.id WHERE cid = $1", - &[&crate_id], - ) - .unwrap(); + &[&crate_id], + )?; crate_details.owners = owners .into_iter() @@ -220,7 +219,7 @@ impl CrateDetails { .next(); } - Some(crate_details) + Ok(Some(crate_details)) } /// Returns the latest non-yanked, non-prerelease release of this crate (or latest @@ -233,10 +232,14 @@ impl CrateDetails { } } -fn releases_for_crate(conn: &mut Client, crate_id: i32) -> Vec { +fn releases_for_crate( + conn: &mut impl GenericClient, + crate_id: i32, +) -> Result, postgres::Error> { let mut releases: Vec = conn .query( "SELECT + id, version, build_status, yanked, @@ -245,13 +248,13 @@ fn releases_for_crate(conn: &mut Client, crate_id: i32) -> Vec { WHERE releases.crate_id = $1", &[&crate_id], - ) - .unwrap() + )? .into_iter() .filter_map(|row| { let version: String = row.get("version"); semver::Version::parse(&version) .map(|semversion| Release { + id: row.get("id"), version: semversion, build_status: row.get("build_status"), yanked: row.get("yanked"), @@ -261,9 +264,8 @@ fn releases_for_crate(conn: &mut Client, crate_id: i32) -> Vec { }) .collect(); - releases.sort_by_key(|r| r.version.clone()); - releases.reverse(); - releases + releases.sort_by(|a, b| b.version.cmp(&a.version)); + Ok(releases) } #[derive(Debug, Clone, PartialEq, Serialize)] @@ -286,7 +288,13 @@ pub fn crate_details_handler(req: &mut Request) -> IronResult { match match_version(&mut conn, name, req_version).and_then(|m| m.assume_exact())? { MatchSemver::Exact((version, _)) => { let updater = extension!(req, RepositoryStatsUpdater); - let details = cexpect!(req, CrateDetails::new(&mut conn, name, &version, updater)); + let details = cexpect!( + req, + ctry!( + req, + CrateDetails::new(&mut *conn, name, &version, Some(updater)) + ) + ); CrateDetailsPage { details }.into_response(req) } @@ -322,13 +330,9 @@ mod tests { version: &str, expected_last_successful_build: Option<&str>, ) -> Result<(), Error> { - let details = CrateDetails::new( - &mut db.conn(), - package, - version, - db.repository_stats_updater(), - ) - .with_context(|| anyhow::anyhow!("could not fetch crate details"))?; + let details = CrateDetails::new(&mut *db.conn(), package, version, None) + .with_context(|| anyhow::anyhow!("could not fetch crate details"))? + .unwrap(); assert_eq!( details.last_successful_build, @@ -455,13 +459,9 @@ mod tests { .binary(true) .create()?; - let details = CrateDetails::new( - &mut db.conn(), - "foo", - "0.2.0", - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", "0.2.0", None) + .unwrap() + .unwrap(); assert_eq!( details.releases, vec![ @@ -470,48 +470,56 @@ mod tests { build_status: true, yanked: false, is_library: true, + id: details.releases[0].id, }, Release { version: semver::Version::parse("0.12.0")?, build_status: true, yanked: false, is_library: true, + id: details.releases[1].id, }, Release { version: semver::Version::parse("0.3.0")?, build_status: false, yanked: false, is_library: true, + id: details.releases[2].id, }, Release { version: semver::Version::parse("0.2.0")?, build_status: true, yanked: true, is_library: true, + id: details.releases[3].id, }, Release { version: semver::Version::parse("0.2.0-alpha")?, build_status: true, yanked: false, is_library: true, + id: details.releases[4].id, }, Release { version: semver::Version::parse("0.1.1")?, build_status: true, yanked: false, is_library: true, + id: details.releases[5].id, }, Release { version: semver::Version::parse("0.1.0")?, build_status: true, yanked: false, is_library: true, + id: details.releases[6].id, }, Release { version: semver::Version::parse("0.0.1")?, build_status: false, yanked: false, is_library: false, + id: details.releases[7].id, }, ] ); @@ -530,13 +538,9 @@ mod tests { env.fake_release().name("foo").version("0.0.2").create()?; for version in &["0.0.1", "0.0.2", "0.0.3"] { - let details = CrateDetails::new( - &mut db.conn(), - "foo", - version, - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", version, None) + .unwrap() + .unwrap(); assert_eq!( details.latest_release().version, semver::Version::parse("0.0.3")? @@ -560,13 +564,9 @@ mod tests { env.fake_release().name("foo").version("0.0.2").create()?; for version in &["0.0.1", "0.0.2", "0.0.3-pre.1"] { - let details = CrateDetails::new( - &mut db.conn(), - "foo", - version, - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", version, None) + .unwrap() + .unwrap(); assert_eq!( details.latest_release().version, semver::Version::parse("0.0.2")? @@ -591,13 +591,9 @@ mod tests { env.fake_release().name("foo").version("0.0.2").create()?; for version in &["0.0.1", "0.0.2", "0.0.3"] { - let details = CrateDetails::new( - &mut db.conn(), - "foo", - version, - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", version, None) + .unwrap() + .unwrap(); assert_eq!( details.latest_release().version, semver::Version::parse("0.0.2")? @@ -630,13 +626,9 @@ mod tests { .create()?; for version in &["0.0.1", "0.0.2", "0.0.3"] { - let details = CrateDetails::new( - &mut db.conn(), - "foo", - version, - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", version, None) + .unwrap() + .unwrap(); assert_eq!( details.latest_release().version, semver::Version::parse("0.0.3")? @@ -692,13 +684,9 @@ mod tests { }) .create()?; - let details = CrateDetails::new( - &mut db.conn(), - "foo", - "0.0.1", - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", "0.0.1", None) + .unwrap() + .unwrap(); assert_eq!( details.owners, vec![("foobar".into(), "https://example.org/foobar".into())] @@ -722,13 +710,9 @@ mod tests { }) .create()?; - let details = CrateDetails::new( - &mut db.conn(), - "foo", - "0.0.1", - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", "0.0.1", None) + .unwrap() + .unwrap(); let mut owners = details.owners; owners.sort(); assert_eq!( @@ -751,13 +735,9 @@ mod tests { }) .create()?; - let details = CrateDetails::new( - &mut db.conn(), - "foo", - "0.0.1", - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", "0.0.1", None) + .unwrap() + .unwrap(); assert_eq!( details.owners, vec![("barfoo".into(), "https://example.org/barfoo".into())] @@ -775,13 +755,9 @@ mod tests { }) .create()?; - let details = CrateDetails::new( - &mut db.conn(), - "foo", - "0.0.1", - db.repository_stats_updater(), - ) - .unwrap(); + let details = CrateDetails::new(&mut *db.conn(), "foo", "0.0.1", None) + .unwrap() + .unwrap(); assert_eq!( details.owners, vec![("barfoo".into(), "https://example.org/barfoov2".into())] diff --git a/src/web/mod.rs b/src/web/mod.rs index 43f9bf5dd..b0d3858f2 100644 --- a/src/web/mod.rs +++ b/src/web/mod.rs @@ -73,7 +73,7 @@ macro_rules! extension { mod build_details; mod builds; -mod crate_details; +pub(crate) mod crate_details; mod csp; mod error; mod extensions; diff --git a/src/web/rustdoc.rs b/src/web/rustdoc.rs index c0d682050..ba21de07c 100644 --- a/src/web/rustdoc.rs +++ b/src/web/rustdoc.rs @@ -328,7 +328,13 @@ pub fn rustdoc_html_server_handler(req: &mut Request) -> IronResult { // Get the crate's details from the database // NOTE: we know this crate must exist because we just checked it above (or else `match_version` is buggy) - let krate = cexpect!(req, CrateDetails::new(&mut conn, &name, &version, updater)); + let krate = cexpect!( + req, + ctry!( + req, + CrateDetails::new(&mut *conn, &name, &version, Some(updater)) + ) + ); // if visiting the full path to the default target, remove the target from the path // expects a req_path that looks like `[/:target]/.*` @@ -543,7 +549,10 @@ pub fn target_redirect_handler(req: &mut Request) -> IronResult { let base = redirect_base(req); let updater = extension!(req, RepositoryStatsUpdater); - let crate_details = match CrateDetails::new(&mut conn, name, version, updater) { + let crate_details = match ctry!( + req, + CrateDetails::new(&mut *conn, name, version, Some(updater)) + ) { Some(krate) => krate, None => return Err(Nope::VersionNotFound.into()), }; From b8dcb8d07c4094f6159874c899d6f3215c4b8f51 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Thu, 18 Nov 2021 18:59:44 +0100 Subject: [PATCH 2/4] use context instead of with_context --- src/db/add_package.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/db/add_package.rs b/src/db/add_package.rs index d57910133..b6c09a2bf 100644 --- a/src/db/add_package.rs +++ b/src/db/add_package.rs @@ -129,7 +129,7 @@ pub(crate) fn add_package_into_database( add_compression_into_database(conn, compression_algorithms.into_iter(), release_id)?; let crate_details = CrateDetails::new(conn, &metadata_pkg.name, &metadata_pkg.version, None) - .with_context(|| anyhow!("error when fetching crate-details"))? + .context("error when fetching crate-details")? .ok_or_else(|| anyhow!("crate details not found directly after creating them"))?; conn.execute( From 75d299fadbd92aac788239a08b1ae73f6027e4d4 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Thu, 18 Nov 2021 19:19:31 +0100 Subject: [PATCH 3/4] change error return type for CrateDetails to anyhow::Error for backtraces --- src/db/migrate.rs | 3 ++- src/web/crate_details.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/db/migrate.rs b/src/db/migrate.rs index 0b71af9d0..3e048c518 100644 --- a/src/db/migrate.rs +++ b/src/db/migrate.rs @@ -786,7 +786,8 @@ pub fn migrate(version: Option, conn: &mut Client) -> crate::error::Res )?; for row in rows.into_iter() { - if let Some(details) = CrateDetails::new(transaction, row.get(0), row.get(1), None)? { + if let Some(details) = CrateDetails::new(transaction, row.get(0), row.get(1), None) + .expect("error when fetching crate details") { transaction.execute( &update_version_query, &[&details.crate_id, &details.latest_release().id], diff --git a/src/web/crate_details.rs b/src/web/crate_details.rs index b8c66d8e7..0f087a52b 100644 --- a/src/web/crate_details.rs +++ b/src/web/crate_details.rs @@ -81,7 +81,7 @@ impl CrateDetails { name: &str, version: &str, up: Option<&RepositoryStatsUpdater>, - ) -> Result, postgres::Error> { + ) -> Result, anyhow::Error> { // get all stuff, I love you rustfmt let query = " SELECT @@ -235,7 +235,7 @@ impl CrateDetails { fn releases_for_crate( conn: &mut impl GenericClient, crate_id: i32, -) -> Result, postgres::Error> { +) -> Result, anyhow::Error> { let mut releases: Vec = conn .query( "SELECT From e6bd26144b2cc0aba1089a39d84e8cb1d86506bd Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sun, 28 Nov 2021 17:07:19 +0100 Subject: [PATCH 4/4] add doc strings back --- src/db/migrate.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/db/migrate.rs b/src/db/migrate.rs index fa04bd534..3b8deb6df 100644 --- a/src/db/migrate.rs +++ b/src/db/migrate.rs @@ -5,6 +5,22 @@ use postgres::{Client, Error as PostgresError, Transaction}; use schemamama::{Migration, Migrator, Version}; use schemamama_postgres::{PostgresAdapter, PostgresMigration}; +/// Creates a new PostgresMigration from upgrade and downgrade closures. +/// Directly using `migration` is only needed for data-migrations or more +/// complex schema-migrations. +/// +/// Example: +/// +/// ``` +/// let my_migration = migration!(100, +/// "migrate data", +/// |transaction: &mut Transaction| -> Result<(), PostgresError> { +/// // upgrade logic here +/// }, +/// |transaction: &mut Transaction| -> Result<(), PostgresError> { +/// // downgrade logic here +/// } ); +/// ``` macro_rules! migration { ($context:expr, $version:expr, $description:expr, $up_func:expr, $down_func:expr $(,)?) => {{ struct Amigration; @@ -52,6 +68,17 @@ macro_rules! migration { }}; } +/// Creates a new PostgresMigration from upgrade and downgrade queries. +/// Downgrade query should return database to previous state. +/// +/// Example: +/// +/// ``` +/// let my_migration = sql_migration!(100, +/// "Create test table", +/// "CREATE TABLE test ( id SERIAL);", +/// "DROP TABLE test;"); +/// ``` macro_rules! sql_migration { ($context:expr, $version:expr, $description:expr, $up:expr, $down:expr $(,)?) => {{ migration!(