diff --git a/src/git.rs b/src/git.rs index d2294cc1e27..45e828ba206 100644 --- a/src/git.rs +++ b/src/git.rs @@ -73,6 +73,10 @@ pub fn add_crate(app: &App, krate: &Crate) -> CargoResult<()> { }) } +/// Yanks or unyanks a crate version. This requires finding the index +/// file, deserlialise the crate from JSON, change the yank boolean to +/// `true` or `false`, write all the lines back out, and commit and +/// push the changes. pub fn yank(app: &App, krate: &str, version: &semver::Version, yanked: bool) -> CargoResult<()> { let repo = app.git_repo.lock().unwrap(); let repo_path = repo.workdir().unwrap(); @@ -112,6 +116,22 @@ pub fn yank(app: &App, krate: &str, version: &semver::Version, yanked: bool) -> }) } +/// Commits and pushes to the crates.io index. +/// +/// There are currently 2 instances of the crates.io backend running +/// on Heroku, and they race against each other e.g. if 2 pushes occur, +/// then one will succeed while the other will need to be rebased before +/// being pushed. +/// +/// A maximum of 20 attempts to commit and push to the index currently +/// accounts for the amount of traffic publishing crates, though this may +/// have to be changed in the future. +/// +/// Notes: +/// Currently, this function is called on the HTTP thread and is blocking. +/// Spawning a separate thread for this function means that the request +/// can return without waiting for completion, and other methods of +/// notifying upon completion or error can be used. fn commit_and_push(repo: &git2::Repository, mut f: F) -> CargoResult<()> where F: FnMut() -> CargoResult<(String, PathBuf)>, diff --git a/src/krate.rs b/src/krate.rs index c8926d0867c..b9445b8d72e 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -785,6 +785,26 @@ impl Model for Crate { } /// Handles the `GET /crates` route. +/// Returns a list of crates. Called in a variety of scenarios in the +/// front end, including: +/// - Alphabetical listing of crates +/// - List of crates under a specific owner +/// - Listing a user's followed crates +/// +/// Notes: +/// The different use cases this function covers is handled through passing +/// in parameters in the GET request. +/// +/// We would like to stop adding functionality in here. It was built like +/// this to keep the number of database queries low, though given Rust's +/// low performance overhead, this is a soft goal to have, and can afford +/// more database transactions if it aids understandability. +/// +/// All of the edge cases for this function are not currently covered +/// in testing, and if they fail, it is difficult to determine what +/// caused the break. In the future, we should look at splitting this +/// function out to cover the different use cases, and create unit tests +/// for them. pub fn index(req: &mut Request) -> CargoResult { use diesel::expression::{AsExpression, DayAndMonthIntervalDsl}; use diesel::types::{Bool, BigInt, Nullable}; @@ -913,6 +933,8 @@ pub fn index(req: &mut Request) -> CargoResult { )); } + // The database query returns a tuple within a tuple , with the root + // tuple containing 3 items. let data = query .paginate(limit, offset) .load::<((Crate, bool, Option), i64)>(&*conn)?; @@ -1100,6 +1122,12 @@ pub fn show(req: &mut Request) -> CargoResult { } /// Handles the `PUT /crates/new` route. +/// Used by `cargo publish` to publish a new crate or to publish a new version of an +/// existing crate. +/// +/// Currently blocks the HTTP thread, perhaps some function calls can spawn new +/// threads and return completion or error through other methods a `cargo publish +/// --status` command, via crates.io's front end, or email. pub fn new(req: &mut Request) -> CargoResult { let app = req.app().clone(); let (new_crate, user) = parse_new_headers(req)?; @@ -1126,6 +1154,8 @@ pub fn new(req: &mut Request) -> CargoResult { let categories: Vec<_> = categories.iter().map(|k| &**k).collect(); let conn = req.db_conn()?; + // Create a transaction on the database, if there are no errors, + // commit the transactions to record a new or updated crate. conn.transaction(|| { // Persist the new crate, if it doesn't already exist let persist = NewCrate { @@ -1236,6 +1266,11 @@ pub fn new(req: &mut Request) -> CargoResult { }) } +/// Used by the `krate::new` function. +/// +/// This function parses the JSON headers to interpret the data and validates +/// the data during and after the parsing. Returns crate metadata and user +/// information. fn parse_new_headers(req: &mut Request) -> CargoResult<(upload::NewCrate, User)> { // Read the json upload request let amt = read_le_u32(req.body())? as u64; diff --git a/src/lib.rs b/src/lib.rs index ff6ef9b471b..0bd2e77ea7a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -148,11 +148,14 @@ pub fn middleware(app: Arc) -> MiddlewareBuilder { C(version::downloads), ); api_router.get("/crates/:crate_id/:version/authors", C(version::authors)); + // Used to generate download graphs api_router.get("/crates/:crate_id/downloads", C(krate::downloads)); api_router.get("/crates/:crate_id/versions", C(krate::versions)); api_router.put("/crates/:crate_id/follow", C(krate::follow)); api_router.delete("/crates/:crate_id/follow", C(krate::unfollow)); api_router.get("/crates/:crate_id/following", C(krate::following)); + // This endpoint may now be redundant, check frontend to see if it is + // being used api_router.get("/crates/:crate_id/owners", C(krate::owners)); api_router.get("/crates/:crate_id/owner_team", C(krate::owner_team)); api_router.get("/crates/:crate_id/owner_user", C(krate::owner_user)); diff --git a/src/upload.rs b/src/upload.rs index b12ebb3bb68..068b4b479f0 100644 --- a/src/upload.rs +++ b/src/upload.rs @@ -1,3 +1,7 @@ +//! This module handles the expected information a crate should have +//! and manages the serialising and deserialising of this information +//! to and from structs. The serlializing is only utilised in +//! integration tests. use std::collections::HashMap; use std::ops::Deref; diff --git a/src/version.rs b/src/version.rs index a2cc6268a84..bb3567294d2 100644 --- a/src/version.rs +++ b/src/version.rs @@ -461,6 +461,14 @@ pub fn authors(req: &mut Request) -> CargoResult { } /// Handles the `DELETE /crates/:crate_id/:version/yank` route. +/// This does not delete a crate version, it makes the crate +/// version accessible only to crates that already have a +/// `Cargo.lock` containing this version. +/// +/// Notes: +/// Crate deletion is not implemented to avoid breaking builds, +/// and the goal of yanking a crate is to prevent crates +/// beginning to depend on the yanked crate version. pub fn yank(req: &mut Request) -> CargoResult { modify_yank(req, true) } @@ -470,6 +478,7 @@ pub fn unyank(req: &mut Request) -> CargoResult { modify_yank(req, false) } +/// Changes `yanked` flag on a crate version record fn modify_yank(req: &mut Request, yanked: bool) -> CargoResult { let (version, krate) = version_and_crate(req)?; let user = req.user()?;