Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ axum = { version = "=0.7.7", features = ["macros", "matched-path"] }
axum-extra = { version = "=0.9.4", features = ["cookie-signed", "typed-header"] }
base64 = "=0.22.1"
bigdecimal = { version = "=0.4.5", features = ["serde"] }
bon = "=2.3.0"
cargo-manifest = "=0.15.2"
crates_io_cdn_logs = { path = "crates/crates_io_cdn_logs" }
crates_io_database = { path = "crates/crates_io_database" }
Expand All @@ -62,7 +63,6 @@ chrono = { version = "=0.4.38", default-features = false, features = ["serde"] }
clap = { version = "=4.5.20", features = ["derive", "env", "unicode", "wrap_help"] }
cookie = { version = "=0.18.1", features = ["secure"] }
deadpool-diesel = { version = "=0.6.1", features = ["postgres", "tracing"] }
derive_builder = "=0.20.2"
derive_deref = "=1.1.1"
dialoguer = "=0.11.0"
diesel = { version = "=2.2.4", features = ["postgres", "serde_json", "chrono", "numeric"] }
Expand Down
11 changes: 5 additions & 6 deletions src/bin/background-worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
use diesel_async::pooled_connection::deadpool::Pool;
use diesel_async::pooled_connection::AsyncDieselConnectionManager;
use object_store::prefix::PrefixStore;
use object_store::ObjectStore;
use reqwest::Client;
use secrecy::ExposeSecret;
use std::sync::Arc;
Expand Down Expand Up @@ -74,7 +73,7 @@
let storage = Arc::new(Storage::from_config(&config.storage));

let downloads_archive_store = PrefixStore::new(storage.as_inner(), "archive/version-downloads");
let downloads_archive_store: Box<dyn ObjectStore> = Box::new(downloads_archive_store);
let downloads_archive_store = Box::new(downloads_archive_store);

Check warning on line 76 in src/bin/background-worker.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/background-worker.rs#L76

Added line #L76 was not covered by tests

let client = Client::builder()
.timeout(Duration::from_secs(45))
Expand All @@ -92,14 +91,14 @@
let environment = Environment::builder()
.config(Arc::new(config))
.repository_config(repository_config)
.cloudfront(cloudfront)
.fastly(fastly)
.maybe_cloudfront(cloudfront)
.maybe_fastly(fastly)

Check warning on line 95 in src/bin/background-worker.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/background-worker.rs#L94-L95

Added lines #L94 - L95 were not covered by tests
.storage(storage)
.downloads_archive_store(Some(downloads_archive_store))
.downloads_archive_store(downloads_archive_store)

Check warning on line 97 in src/bin/background-worker.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/background-worker.rs#L97

Added line #L97 was not covered by tests
.deadpool(deadpool.clone())
.emails(emails)
.team_repo(Box::new(team_repo))
.build()?;
.build();

Check warning on line 101 in src/bin/background-worker.rs

View check run for this annotation

Codecov / codecov/patch

src/bin/background-worker.rs#L101

Added line #L101 was not covered by tests

let environment = Arc::new(environment);

Expand Down
9 changes: 4 additions & 5 deletions src/controllers/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,19 +360,18 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra

// Persist the new version of this crate
let version = NewVersion::builder(krate.id, &version_string)
.features(&features)?
.license(license.as_deref())
.features(serde_json::to_value(&features)?)
.maybe_license(license.as_deref())
// Downcast is okay because the file length must be less than the max upload size
// to get here, and max upload sizes are way less than i32 max
.size(content_length as i32)
.published_by(user.id)
.checksum(&hex_cksum)
.links(package.links.as_deref())
.rust_version(rust_version.as_deref())
.maybe_links(package.links.as_deref())
.maybe_rust_version(rust_version.as_deref())
.has_lib(tarball_info.manifest.lib.is_some())
.bin_names(bin_names.as_slice())
.build()
.map_err(|error| internal(error.to_string()))?
.save(conn, &verified_email_address)?;

insert_version_owner_action(
Expand Down
42 changes: 5 additions & 37 deletions src/models/version.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use std::collections::BTreeMap;

use bon::Builder;
use chrono::NaiveDateTime;
use crates_io_index::features::FeaturesMap;
use derive_builder::Builder;
use diesel::prelude::*;
use serde::Deserialize;

Expand Down Expand Up @@ -80,58 +80,26 @@ impl Version {
#[derive(Insertable, Debug, Builder)]
#[diesel(table_name = versions, check_for_backend(diesel::pg::Pg))]
pub struct NewVersion<'a> {
#[builder(start_fn)]
crate_id: i32,
#[builder(start_fn)]
num: &'a str,
#[builder(default)]
created_at: Option<&'a NaiveDateTime>,
#[builder(default, setter(strip_option))]
yanked: Option<bool>,
#[builder(
default = "serde_json::Value::Object(Default::default())",
setter(custom)
)]
#[builder(default = serde_json::Value::Object(Default::default()))]
features: serde_json::Value,
#[builder(default)]
license: Option<&'a str>,
#[builder(default, setter(name = "size"))]
#[builder(default, name = "size")]
crate_size: i32,
published_by: i32,
checksum: &'a str,
#[builder(default)]
links: Option<&'a str>,
#[builder(default)]
rust_version: Option<&'a str>,
#[builder(default, setter(strip_option))]
pub has_lib: Option<bool>,
#[builder(default, setter(strip_option))]
pub bin_names: Option<&'a [&'a str]>,
}

impl NewVersionBuilder<'_> {
pub fn features(
&mut self,
features: &BTreeMap<String, Vec<String>>,
) -> serde_json::Result<&mut Self> {
self.features = Some(serde_json::to_value(features)?);
Ok(self)
}

Comment on lines -111 to -118
Copy link

@Veetaha Veetaha Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI there will be a new attribute in 3.0. that will allow expressing this setter as this (apart from defining the custom method on the builder directly):

#[builder(with = |features: &BTreeMap<...>| -> serde_json::Result<_> { 
  serde_json::to_value(features) 
})]
features: serde_json::Value,

Yes it's possible to have a fallible setter this way if you annotate the return type of the closure with a type that has Result suffix. The lack of return type annotation implies the closure passed to with is infallible.

/// Set the `checksum` field to a basic dummy value.
pub fn dummy_checksum(&mut self) -> &mut Self {
const DUMMY_CHECKSUM: &str =
"0000000000000000000000000000000000000000000000000000000000000000";

self.checksum(DUMMY_CHECKSUM)
}
}

impl NewVersion<'_> {
pub fn builder(crate_id: i32, version: &str) -> NewVersionBuilder<'_> {
let mut builder = NewVersionBuilder::default();
builder.crate_id(crate_id).num(version);
builder
}

pub fn save(&self, conn: &mut impl Conn, published_by_email: &str) -> AppResult<Version> {
use diesel::dsl::exists;
use diesel::{insert_into, select};
Expand Down
14 changes: 6 additions & 8 deletions src/tests/builders/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::{
};
use std::collections::BTreeMap;

use crate::util::errors::internal;
use chrono::NaiveDateTime;
use diesel::prelude::*;

Expand Down Expand Up @@ -102,17 +101,16 @@ impl VersionBuilder {
let version = self.num.to_string();

let new_version = NewVersion::builder(crate_id, &version)
.features(&self.features)?
.license(self.license.as_deref())
.features(serde_json::to_value(&self.features)?)
.maybe_license(self.license.as_deref())
.size(self.size)
.published_by(published_by)
.checksum(&self.checksum)
.links(self.links.as_deref())
.rust_version(self.rust_version.as_deref())
.maybe_links(self.links.as_deref())
.maybe_rust_version(self.rust_version.as_deref())
.yanked(self.yanked)
.created_at(self.created_at.as_ref())
.build()
.map_err(|error| internal(error.to_string()))?;
.maybe_created_at(self.created_at.as_ref())
.build();

let vers = new_version.save(connection, "[email protected]")?;

Expand Down
3 changes: 1 addition & 2 deletions src/tests/util/test_app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,7 @@ impl TestAppBuilder {
.deadpool(app.primary_database.clone())
.emails(app.emails.clone())
.team_repo(Box::new(self.team_repo))
.build()
.unwrap();
.build();

let runner = Runner::new(app.primary_database.clone(), Arc::new(environment))
.shutdown_when_queue_empty()
Expand Down
3 changes: 1 addition & 2 deletions src/typosquat/test_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ pub mod faker {

let version = NewVersion::builder(krate.id, "1.0.0")
.published_by(user.id)
.dummy_checksum()
.checksum("0000000000000000000000000000000000000000000000000000000000000000")
.build()
.unwrap()
.save(conn, "[email protected]")
.unwrap();

Expand Down
14 changes: 3 additions & 11 deletions src/worker/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ use crate::typosquat;
use crate::util::diesel::Conn;
use crate::Emails;
use anyhow::Context;
use bon::Builder;
use crates_io_index::{Repository, RepositoryConfig};
use crates_io_team_repo::TeamRepo;
use derive_builder::Builder;
use diesel_async::pooled_connection::deadpool::Pool;
use diesel_async::AsyncPgConnection;
use object_store::ObjectStore;
Expand All @@ -17,34 +17,26 @@ use std::sync::{Arc, OnceLock};
use std::time::Instant;

#[derive(Builder)]
#[builder(pattern = "owned")]
pub struct Environment {
pub config: Arc<crate::config::Server>,

repository_config: RepositoryConfig,
#[builder(default, setter(skip))]
#[builder(skip)]
repository: Mutex<Option<Repository>>,
#[builder(default)]
cloudfront: Option<CloudFront>,
#[builder(default)]
fastly: Option<Fastly>,
pub storage: Arc<Storage>,
#[builder(default)]
pub downloads_archive_store: Option<Box<dyn ObjectStore>>,
pub deadpool: Pool<AsyncPgConnection>,
pub emails: Emails,
pub team_repo: Box<dyn TeamRepo + Send + Sync>,

/// A lazily initialised cache of the most popular crates ready to use in typosquatting checks.
#[builder(default, setter(skip))]
#[builder(skip)]
typosquat_cache: OnceLock<Result<typosquat::Cache, typosquat::CacheError>>,
}

impl Environment {
pub fn builder() -> EnvironmentBuilder {
EnvironmentBuilder::default()
}

#[instrument(skip_all)]
pub fn lock_index(&self) -> anyhow::Result<RepositoryLock<'_>> {
let mut repo = self.repository.lock();
Expand Down
5 changes: 2 additions & 3 deletions src/worker/jobs/downloads/update_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,8 @@ mod tests {

let version = NewVersion::builder(krate.id, "1.0.0")
.published_by(user_id)
.dummy_checksum()
.build()
.unwrap();
.checksum("0000000000000000000000000000000000000000000000000000000000000000")
.build();

let version = version.save(conn, "[email protected]").unwrap();
(krate, version)
Expand Down