Skip to content

Convert more endpoints to the read-only follower database #3118

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
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
6 changes: 3 additions & 3 deletions src/controllers/category.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
let offset = options.offset().unwrap_or_default();
let sort = query.get("sort").map_or("alpha", String::as_str);

let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let categories =
Category::toplevel(&conn, sort, i64::from(options.per_page), i64::from(offset))?;
let categories = categories.into_iter().map(Category::encodable).collect();
Expand All @@ -42,7 +42,7 @@ pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
/// Handles the `GET /categories/:category_id` route.
pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
let slug = &req.params()["category_id"];
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let cat: Category = Category::by_slug(slug).first(&*conn)?;
let subcats = cat
.subcategories(&conn)?
Expand Down Expand Up @@ -78,7 +78,7 @@ pub fn show(req: &mut dyn RequestExt) -> EndpointResult {

/// Handles the `GET /category_slugs` route.
pub fn slugs(req: &mut dyn RequestExt) -> EndpointResult {
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let slugs = categories::table
.select((categories::slug, categories::slug, categories::description))
.order(categories::slug)
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/crate_owner_invitation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::views::{EncodableCrateOwnerInvitation, InvitationResponse};
/// Handles the `GET /me/crate_owner_invitations` route.
pub fn list(req: &mut dyn RequestExt) -> EndpointResult {
let user_id = req.authenticate()?.user_id();
let conn = &*req.db_conn()?;
let conn = &*req.db_read_only()?;

let crate_owner_invitations: Vec<CrateOwnerInvitation> = crate_owner_invitations::table
.filter(crate_owner_invitations::invited_user_id.eq(user_id))
Expand Down
4 changes: 2 additions & 2 deletions src/controllers/keyword.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
}

let query = query.paginate(req)?;
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let data: Paginated<Keyword> = query.load(&*conn)?;
let total = data.total();
let kws = data.into_iter().map(Keyword::encodable).collect::<Vec<_>>();
Expand All @@ -44,7 +44,7 @@ pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
/// Handles the `GET /keywords/:keyword_id` route.
pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
let name = &req.params()["keyword_id"];
let conn = req.db_conn()?;
let conn = req.db_read_only()?;

let kw = Keyword::find_by_keyword(&conn, name)?;

Expand Down
2 changes: 1 addition & 1 deletion src/controllers/krate/follow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub fn following(req: &mut dyn RequestExt) -> EndpointResult {
use diesel::dsl::exists;

let user_id = req.authenticate()?.user_id();
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let follow = follow_target(req, &conn, user_id)?;
let following = diesel::select(exists(follows::table.find(follow.id()))).get_result(&*conn)?;

Expand Down
6 changes: 3 additions & 3 deletions src/controllers/krate/owners.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::views::EncodableOwner;
/// Handles the `GET /crates/:crate_id/owners` route.
pub fn owners(req: &mut dyn RequestExt) -> EndpointResult {
let crate_name = &req.params()["crate_id"];
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
let owners = krate
.owners(&conn)?
Expand All @@ -25,7 +25,7 @@ pub fn owners(req: &mut dyn RequestExt) -> EndpointResult {
/// Handles the `GET /crates/:crate_id/owner_team` route.
pub fn owner_team(req: &mut dyn RequestExt) -> EndpointResult {
let crate_name = &req.params()["crate_id"];
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
let owners = Team::owning(&krate, &conn)?
.into_iter()
Expand All @@ -42,7 +42,7 @@ pub fn owner_team(req: &mut dyn RequestExt) -> EndpointResult {
/// Handles the `GET /crates/:crate_id/owner_user` route.
pub fn owner_user(req: &mut dyn RequestExt) -> EndpointResult {
let crate_name = &req.params()["crate_id"];
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
let owners = User::owning(&krate, &conn)?
.into_iter()
Expand Down
2 changes: 1 addition & 1 deletion src/controllers/team.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub fn show_team(req: &mut dyn RequestExt) -> EndpointResult {
use self::teams::dsl::{login, teams};

let name = &req.params()["team_id"];
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let team: Team = teams.filter(login.eq(name)).first(&*conn)?;

#[derive(Serialize)]
Expand Down
21 changes: 13 additions & 8 deletions src/controllers/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,23 @@ pub mod yank;

use super::prelude::*;

use crate::db::DieselPooledConn;
use crate::models::{Crate, Version};

fn version_and_crate(req: &dyn RequestExt) -> AppResult<(DieselPooledConn<'_>, Version, Crate)> {
let crate_name = extract_crate_name(req);
let semver = extract_semver(req)?;

let conn = req.db_conn()?;
let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
fn version_and_crate(
conn: &PgConnection,
crate_name: &str,
semver: &str,
) -> AppResult<(Version, Crate)> {
let krate: Crate = Crate::by_name(crate_name).first(conn)?;
let version = krate.find_version(&conn, semver)?;

Ok((conn, version, krate))
Ok((version, krate))
}

fn extract_crate_name_and_semver(req: &dyn RequestExt) -> AppResult<(&str, &str)> {
let name = extract_crate_name(req);
let version = extract_semver(req)?;
Ok((name, version))
}

fn extract_crate_name(req: &dyn RequestExt) -> &str {
Expand Down
4 changes: 2 additions & 2 deletions src/controllers/version/deprecated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::views::EncodableVersion;
/// Handles the `GET /versions` route.
pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
use diesel::dsl::any;
let conn = req.db_conn()?;
let conn = req.db_read_only()?;

// Extract all ids requested.
let query = url::form_urlencoded::parse(req.query_string().unwrap_or("").as_bytes());
Expand Down Expand Up @@ -58,7 +58,7 @@ pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
pub fn show_by_id(req: &mut dyn RequestExt) -> EndpointResult {
let id = &req.params()["version_id"];
let id = id.parse().unwrap_or(0);
let conn = req.db_conn()?;
let conn = req.db_read_only()?;
let (version, krate, published_by): (Version, Crate, Option<User>) = versions::table
.find(id)
.inner_join(crates::table)
Expand Down
8 changes: 3 additions & 5 deletions src/controllers/version/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::models::{Crate, VersionDownload};
use crate::schema::*;
use crate::views::EncodableVersionDownload;

use super::{extract_crate_name, extract_semver};
use super::{extract_crate_name_and_semver, version_and_crate};

/// Handles the `GET /crates/:crate_id/:version/download` route.
/// This returns a URL to the location where the crate is stored.
Expand Down Expand Up @@ -75,12 +75,10 @@ fn increment_download_counts(

/// Handles the `GET /crates/:crate_id/:version/downloads` route.
pub fn downloads(req: &mut dyn RequestExt) -> EndpointResult {
let crate_name = extract_crate_name(req);
let semver = extract_semver(req)?;
let (crate_name, semver) = extract_crate_name_and_semver(req)?;

let conn = req.db_read_only()?;
let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
let version = krate.find_version(&conn, semver)?;
let (version, _) = version_and_crate(&conn, crate_name, semver)?;

let cutoff_end_date = req
.query()
Expand Down
16 changes: 11 additions & 5 deletions src/controllers/version/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::models::VersionOwnerAction;
use crate::schema::*;
use crate::views::{EncodableDependency, EncodablePublicUser, EncodableVersion};

use super::version_and_crate;
use super::{extract_crate_name_and_semver, version_and_crate};

/// Handles the `GET /crates/:crate_id/:version/dependencies` route.
///
Expand All @@ -20,8 +20,10 @@ use super::version_and_crate;
/// fields for `id`, `version_id`, and `downloads` (which appears to always
/// be 0)
pub fn dependencies(req: &mut dyn RequestExt) -> EndpointResult {
let (conn, version, _) = version_and_crate(req)?;
let deps = version.dependencies(&*conn)?;
let (crate_name, semver) = extract_crate_name_and_semver(req)?;
let conn = req.db_read_only()?;
let (version, _) = version_and_crate(&conn, crate_name, semver)?;
let deps = version.dependencies(&conn)?;
let deps = deps
.into_iter()
.map(|(dep, crate_name)| dep.encodable(&crate_name, None))
Expand All @@ -36,7 +38,9 @@ pub fn dependencies(req: &mut dyn RequestExt) -> EndpointResult {

/// Handles the `GET /crates/:crate_id/:version/authors` route.
pub fn authors(req: &mut dyn RequestExt) -> EndpointResult {
let (conn, version, _) = version_and_crate(req)?;
let (crate_name, semver) = extract_crate_name_and_semver(req)?;
let conn = req.db_read_only()?;
let (version, _) = version_and_crate(&conn, crate_name, semver)?;
let names = version_authors::table
.filter(version_authors::version_id.eq(version.id))
.select(version_authors::name)
Expand Down Expand Up @@ -66,7 +70,9 @@ pub fn authors(req: &mut dyn RequestExt) -> EndpointResult {
/// The frontend doesn't appear to hit this endpoint, but our tests do, and it seems to be a useful
/// API route to have.
pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
let (conn, version, krate) = version_and_crate(req)?;
let (crate_name, semver) = extract_crate_name_and_semver(req)?;
let conn = req.db_read_only()?;
let (version, krate) = version_and_crate(&conn, crate_name, semver)?;
let published_by = version.published_by(&conn);
let actions = VersionOwnerAction::by_version(&conn, &version)?;

Expand Down
9 changes: 7 additions & 2 deletions src/controllers/version/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

use swirl::Job;

use super::version_and_crate;
use super::{extract_crate_name_and_semver, version_and_crate};
use crate::controllers::cargo_prelude::*;
use crate::git;
use crate::models::Rights;
Expand All @@ -28,8 +28,13 @@ pub fn unyank(req: &mut dyn RequestExt) -> EndpointResult {

/// Changes `yanked` flag on a crate version record
fn modify_yank(req: &mut dyn RequestExt, yanked: bool) -> EndpointResult {
// FIXME: Should reject bad requests before authentication, but can't due to
// lifetime issues with `req`.
let authenticated_user = req.authenticate()?;
let (conn, version, krate) = version_and_crate(req)?;
let (crate_name, semver) = extract_crate_name_and_semver(req)?;

let conn = req.db_conn()?;
let (version, krate) = version_and_crate(&conn, crate_name, semver)?;
let api_token_id = authenticated_user.api_token_id();
let user = authenticated_user.user();
let owners = krate.owners(&conn)?;
Expand Down