Convert more endpoints to the read-only follower database

Author: jtgeibel

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