Skip to content

Update owner details more fully during build and add CLI command to trigger it #917

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

Merged
merged 9 commits into from
Jul 31, 2020
41 changes: 20 additions & 21 deletions src/bin/cratesfyi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::path::PathBuf;
use std::sync::Arc;

use cratesfyi::db::{self, add_path_into_database, Pool};
use cratesfyi::index::Index;
use cratesfyi::utils::{remove_crate_priority, set_crate_priority};
use cratesfyi::{
BuildQueue, Config, DocBuilder, DocBuilderOptions, RustwideBuilder, Server, Storage,
Expand Down Expand Up @@ -241,22 +242,6 @@ impl PrioritySubcommand {
#[derive(Debug, Clone, PartialEq, Eq, StructOpt)]
#[structopt(rename_all = "kebab-case")]
struct Build {
#[structopt(
name = "PREFIX",
short = "P",
long = "prefix",
env = "CRATESFYI_PREFIX"
)]
prefix: PathBuf,

/// Sets the registry index path, where on disk the registry index will be cloned to
#[structopt(
name = "REGISTRY_INDEX_PATH",
long = "registry-index-path",
alias = "crates-io-index-path"
)]
registry_index_path: Option<PathBuf>,

/// Skips building documentation if documentation exists
#[structopt(name = "SKIP_IF_EXISTS", short = "s", long = "skip")]
skip_if_exists: bool,
Expand All @@ -280,11 +265,9 @@ struct Build {
impl Build {
pub fn handle_args(self, ctx: Context) -> Result<(), Error> {
let docbuilder = {
let mut doc_options = DocBuilderOptions::from_prefix(self.prefix);

if let Some(registry_index_path) = self.registry_index_path {
doc_options.registry_index_path = registry_index_path;
}
let config = ctx.config()?;
let mut doc_options =
DocBuilderOptions::new(config.prefix.clone(), config.registry_index_path.clone());

doc_options.skip_if_exists = self.skip_if_exists;
doc_options.skip_if_log_exists = self.skip_if_log_exists;
Expand Down Expand Up @@ -435,6 +418,12 @@ enum DatabaseSubcommand {
/// Updates github stats for crates.
UpdateGithubFields,

/// Updates info for a crate from the registry's API
UpdateCrateRegistryFields {
#[structopt(name = "CRATE")]
name: String,
},

AddDirectory {
/// Path of file or directory
#[structopt(name = "DIRECTORY")]
Expand Down Expand Up @@ -472,6 +461,16 @@ impl DatabaseSubcommand {
.update_all_crates()?;
}

Self::UpdateCrateRegistryFields { name } => {
let index = Index::new(&ctx.config()?.registry_index_path)?;

db::update_crate_data_in_database(
&*ctx.conn()?,
&name,
&index.api().get_crate_data(&name)?,
)?;
}

Self::AddDirectory { directory, prefix } => {
add_path_into_database(&*ctx.storage()?, &prefix, directory)
.context("Failed to add directory into database")?;
Expand Down
9 changes: 9 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
use failure::{bail, format_err, Error, Fail, ResultExt};
use std::env::VarError;
use std::path::PathBuf;
use std::str::FromStr;

#[derive(Debug)]
pub struct Config {
// Build params
pub(crate) build_attempts: u16,

pub prefix: PathBuf,
pub registry_index_path: PathBuf,

// Database connection params
pub(crate) database_url: String,
pub(crate) max_pool_size: u32,
Expand All @@ -23,9 +27,14 @@ pub struct Config {

impl Config {
pub fn from_env() -> Result<Self, Error> {
let prefix: PathBuf = require_env("CRATESFYI_PREFIX")?;

Ok(Self {
build_attempts: env("DOCSRS_BUILD_ATTEMPTS", 5)?,

prefix: prefix.clone(),
registry_index_path: env("REGISTRY_INDEX_PATH", prefix.join("crates.io-index"))?,

database_url: require_env("CRATESFYI_DATABASE_URL")?,
max_pool_size: env("DOCSRS_MAX_POOL_SIZE", 90)?,
min_pool_idle: env("DOCSRS_MIN_POOL_IDLE", 10)?,
Expand Down
98 changes: 77 additions & 21 deletions src/db/add_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@ use std::{
use crate::{
docbuilder::BuildResult,
error::Result,
index::api::{CrateOwner, RegistryCrateData},
index::api::{CrateData, CrateOwner, ReleaseData},
storage::CompressionAlgorithm,
utils::MetadataPackage,
};
use log::debug;
use log::{debug, info};
use postgres::Connection;
use regex::Regex;
use serde_json::Value;
Expand All @@ -32,7 +32,7 @@ pub(crate) fn add_package_into_database(
default_target: &str,
source_files: Option<Value>,
doc_targets: Vec<String>,
registry_data: &RegistryCrateData,
registry_data: &ReleaseData,
has_docs: bool,
has_examples: bool,
compression_algorithms: std::collections::HashSet<CompressionAlgorithm>,
Expand Down Expand Up @@ -117,7 +117,6 @@ pub(crate) fn add_package_into_database(

add_keywords_into_database(&conn, &metadata_pkg, release_id)?;
add_authors_into_database(&conn, &metadata_pkg, release_id)?;
add_owners_into_database(&conn, &registry_data.owners, crate_id)?;
add_compression_into_database(&conn, compression_algorithms.into_iter(), release_id)?;

// Update the crates table with the new release
Expand Down Expand Up @@ -328,31 +327,88 @@ fn add_authors_into_database(
Ok(())
}

pub fn update_crate_data_in_database(
conn: &Connection,
name: &str,
registry_data: &CrateData,
) -> Result<()> {
info!("Updating crate data for {}", name);
let crate_id = conn
.query("SELECT id FROM crates WHERE crates.name = $1", &[&name])?
.get(0)
.get(0);

update_owners_in_database(conn, &registry_data.owners, crate_id)?;

Ok(())
}

/// Adds owners into database
fn add_owners_into_database(conn: &Connection, owners: &[CrateOwner], crate_id: i32) -> Result<()> {
fn update_owners_in_database(
conn: &Connection,
owners: &[CrateOwner],
crate_id: i32,
) -> Result<()> {
let rows = conn.query(
"
SELECT login
FROM owners
INNER JOIN owner_rels
ON owner_rels.oid = owners.id
WHERE owner_rels.cid = $1
",
&[&crate_id],
)?;
let existing_owners = rows.into_iter().map(|row| -> String { row.get(0) });

for owner in owners {
debug!("Updating owner data for {}: {:?}", owner.login, owner);

// Update any existing owner data since it is mutable and could have changed since last
// time we pulled it
let owner_id: i32 = {
let rows = conn.query("SELECT id FROM owners WHERE login = $1", &[&owner.login])?;
if !rows.is_empty() {
rows.get(0).get(0)
} else {
conn.query(
"INSERT INTO owners (login, avatar, name, email)
VALUES ($1, $2, $3, $4)
RETURNING id",
&[&owner.login, &owner.avatar, &owner.name, &owner.email],
)?
.get(0)
.get(0)
}
conn.query(
"
INSERT INTO owners (login, avatar, name, email)
VALUES ($1, $2, $3, $4)
ON CONFLICT (login) DO UPDATE
SET
avatar = $2,
name = $3,
email = $4
RETURNING id
",
&[&owner.login, &owner.avatar, &owner.name, &owner.email],
)?
.get(0)
.get(0)
};

// add relationship
let _ = conn.query(
"INSERT INTO owner_rels (cid, oid) VALUES ($1, $2)",
conn.query(
"INSERT INTO owner_rels (cid, oid) VALUES ($1, $2) ON CONFLICT DO NOTHING",
&[&crate_id, &owner_id],
);
)?;
}

let to_remove =
existing_owners.filter(|login| !owners.iter().any(|owner| &owner.login == login));

for login in to_remove {
debug!("Removing owner relationship {}", login);
// remove relationship
conn.query(
"
DELETE FROM owner_rels
USING owners
WHERE owner_rels.cid = $1
AND owner_rels.oid = owners.id
AND owners.login = $2
",
&[&crate_id, &login],
)?;
}

Ok(())
}

Expand Down
4 changes: 2 additions & 2 deletions src/db/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Database operations

pub(crate) use self::add_package::add_build_into_database;
pub(crate) use self::add_package::add_package_into_database;
pub use self::add_package::update_crate_data_in_database;
pub(crate) use self::add_package::{add_build_into_database, add_package_into_database};
pub use self::delete::{delete_crate, delete_version};
pub use self::file::add_path_into_database;
pub use self::migrate::migrate;
Expand Down
47 changes: 3 additions & 44 deletions src/docbuilder/options.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use crate::error::Result;
use std::path::PathBuf;
use std::{env, fmt};

#[derive(Clone)]
#[derive(Clone, Debug)]
pub struct DocBuilderOptions {
pub keep_build_directory: bool,
pub prefix: PathBuf,
Expand All @@ -14,12 +13,8 @@ pub struct DocBuilderOptions {
pub debug: bool,
}

impl Default for DocBuilderOptions {
fn default() -> DocBuilderOptions {
let cwd = env::current_dir().unwrap();

let (prefix, registry_index_path) = generate_paths(cwd);

impl DocBuilderOptions {
pub fn new(prefix: PathBuf, registry_index_path: PathBuf) -> DocBuilderOptions {
DocBuilderOptions {
prefix,
registry_index_path,
Expand All @@ -32,36 +27,6 @@ impl Default for DocBuilderOptions {
debug: false,
}
}
}

impl fmt::Debug for DocBuilderOptions {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
"DocBuilderOptions {{ \
registry_index_path: {:?}, \
keep_build_directory: {:?}, skip_if_exists: {:?}, \
skip_if_log_exists: {:?}, debug: {:?} }}",
self.registry_index_path,
self.keep_build_directory,
self.skip_if_exists,
self.skip_if_log_exists,
self.debug
)
}
}

impl DocBuilderOptions {
/// Creates new DocBuilderOptions from prefix
pub fn from_prefix(prefix: PathBuf) -> DocBuilderOptions {
let (prefix, registry_index_path) = generate_paths(prefix);

DocBuilderOptions {
prefix,
registry_index_path,
..Default::default()
}
}

pub fn check_paths(&self) -> Result<()> {
if !self.registry_index_path.exists() {
Expand All @@ -74,9 +39,3 @@ impl DocBuilderOptions {
Ok(())
}
}

fn generate_paths(prefix: PathBuf) -> (PathBuf, PathBuf) {
let registry_index_path = PathBuf::from(&prefix).join("crates.io-index");

(prefix, registry_index_path)
}
23 changes: 21 additions & 2 deletions src/docbuilder/rustwide_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ use super::DocBuilder;
use super::Metadata;
use crate::db::blacklist::is_blacklisted;
use crate::db::file::add_path_into_database;
use crate::db::{add_build_into_database, add_package_into_database, Pool};
use crate::db::{
add_build_into_database, add_package_into_database, update_crate_data_in_database, Pool,
};
use crate::docbuilder::{crates::crates_from_path, Limits};
use crate::error::Result;
use crate::index::api::ReleaseData;
use crate::storage::CompressionAlgorithms;
use crate::storage::Storage;
use crate::utils::{copy_doc_dir, parse_rustc_version, CargoMetadata};
Expand Down Expand Up @@ -397,6 +400,15 @@ impl RustwideBuilder {
} else {
crate::web::metrics::NON_LIBRARY_BUILDS.inc();
}

let release_data = match doc_builder.index.api().get_release_data(name, version) {
Ok(data) => data,
Err(err) => {
warn!("{:#?}", err);
ReleaseData::default()
}
};

let release_id = add_package_into_database(
&conn,
res.cargo_metadata.root(),
Expand All @@ -405,13 +417,20 @@ impl RustwideBuilder {
&res.target,
files_list,
successful_targets,
&doc_builder.index.api().get_crate_data(name, version),
&release_data,
has_docs,
has_examples,
algs,
)?;

add_build_into_database(&conn, release_id, &res.result)?;

// Some crates.io crate data is mutable, so we proactively update it during a release
match doc_builder.index.api().get_crate_data(name) {
Ok(crate_data) => update_crate_data_in_database(&conn, name, &crate_data)?,
Err(err) => warn!("{:#?}", err),
}

doc_builder.add_to_cache(name, version);
Ok(res)
})?;
Expand Down
Loading