Auto merge of #3118 - jtgeibel:more-endpoints-on-readonly-follower-db, r=Turbo87

Convert more endpoints to the read-only follower database

r? `@pietroalbini`

Author: bors

src/controllers/category.rs

@@ -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::into).collect();
@@ -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)?
@@ -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)

src/controllers/crate_owner_invitation.rs

@@ -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))

src/controllers/keyword.rs

@@ -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::into).collect::<Vec<_>>();
@@ -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)?;
 

src/controllers/krate/follow.rs

@@ -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)?;
 

src/controllers/krate/owners.rs

@@ -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)?.into_iter().map(Owner::into).collect();
 
@@ -21,7 +21,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()
@@ -38,7 +38,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()

src/controllers/team.rs

@@ -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)]

src/controllers/version.rs

@@ -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 {

src/controllers/version/deprecated.rs

@@ -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());
@@ -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)

src/controllers/version/downloads.rs

@@ -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.
@@ -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()

src/controllers/version/metadata.rs

@@ -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.
 ///
@@ -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))
@@ -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)
@@ -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)?;
 

src/controllers/version/yank.rs

@@ -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;
@@ -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)?;