Extract RequestParamExt::param() extension fn (#5801)

Turbo87 · web-flow · commit 802f7ec392cd · 2023-01-03T21:32:49.000+01:00
This simplifies how we access path parameters and prepares for adding support for `axum`
diff --git a/src/controllers.rs b/src/controllers.rs
@@ -10,10 +10,10 @@ mod frontend_prelude {
 
 mod prelude {
     pub use super::helpers::ok_true;
+    pub use super::util::RequestParamExt;
     pub use diesel::prelude::*;
 
     pub use conduit::RequestExt;
-    pub use conduit_router::RequestParams;
     pub use http::{header, StatusCode};
 
     pub use super::conduit_axum::conduit_compat;
diff --git a/src/controllers/category.rs b/src/controllers/category.rs
@@ -34,7 +34,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 slug = req.param("category_id").unwrap();
     let conn = req.app().db_read()?;
     let cat: Category = Category::by_slug(slug).first(&*conn)?;
     let subcats = cat
diff --git a/src/controllers/crate_owner_invitation.rs b/src/controllers/crate_owner_invitation.rs
@@ -282,7 +282,7 @@ pub fn handle_invite_with_token(req: &mut dyn RequestExt) -> EndpointResult {
     let config = &state.config;
     let conn = state.db_write()?;
 
-    let req_token = &req.params()["token"];
+    let req_token = req.param("token").unwrap();
 
     let invitation = CrateOwnerInvitation::find_by_token(req_token, &conn)?;
     let crate_id = invitation.crate_id;
diff --git a/src/controllers/krate/downloads.rs b/src/controllers/krate/downloads.rs
@@ -17,7 +17,7 @@ pub fn downloads(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::dsl::*;
     use diesel::sql_types::BigInt;
 
-    let crate_name = &req.params()["crate_id"];
+    let crate_name = req.param("crate_id").unwrap();
     let conn = req.app().db_read()?;
     let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
 
diff --git a/src/controllers/krate/follow.rs b/src/controllers/krate/follow.rs
@@ -13,7 +13,7 @@ fn follow_target(
     conn: &DieselPooledConn<'_>,
     user_id: i32,
 ) -> AppResult<Follow> {
-    let crate_name = &req.params()["crate_id"];
+    let crate_name = req.param("crate_id").unwrap();
     let crate_id = Crate::by_name(crate_name)
         .select(crates::id)
         .first(&**conn)?;
diff --git a/src/controllers/krate/metadata.rs b/src/controllers/krate/metadata.rs
@@ -126,7 +126,7 @@ pub fn summary(req: &mut dyn RequestExt) -> EndpointResult {
 
 /// Handles the `GET /crates/:crate_id` route.
 pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
-    let name = &req.params()["crate_id"];
+    let name = req.param("crate_id").unwrap();
     let include = req
         .query()
         .get("include")
@@ -299,8 +299,8 @@ impl FromStr for ShowIncludeMode {
 
 /// Handles the `GET /crates/:crate_id/:version/readme` route.
 pub fn readme(req: &mut dyn RequestExt) -> EndpointResult {
-    let crate_name = &req.params()["crate_id"];
-    let version = &req.params()["version"];
+    let crate_name = req.param("crate_id").unwrap();
+    let version = req.param("version").unwrap();
 
     let redirect_url = req
         .app()
@@ -319,7 +319,7 @@ pub fn readme(req: &mut dyn RequestExt) -> EndpointResult {
 // FIXME: Not sure why this is necessary since /crates/:crate_id returns
 // this information already, but ember is definitely requesting it
 pub fn versions(req: &mut dyn RequestExt) -> EndpointResult {
-    let crate_name = &req.params()["crate_id"];
+    let crate_name = req.param("crate_id").unwrap();
     let conn = req.app().db_read()?;
     let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
     let mut versions_and_publishers: Vec<(Version, Option<User>)> = krate
@@ -350,7 +350,7 @@ pub fn reverse_dependencies(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::dsl::any;
 
     let pagination_options = PaginationOptions::builder().gather(req)?;
-    let name = &req.params()["crate_id"];
+    let name = req.param("crate_id").unwrap();
     let conn = req.app().db_read()?;
     let krate: Crate = Crate::by_name(name).first(&*conn)?;
     let (rev_deps, total) = krate.reverse_dependencies(&conn, pagination_options)?;
diff --git a/src/controllers/krate/owners.rs b/src/controllers/krate/owners.rs
@@ -8,7 +8,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 crate_name = req.param("crate_id").unwrap();
     let conn = req.app().db_read()?;
     let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
     let owners = krate
@@ -22,7 +22,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 crate_name = req.param("crate_id").unwrap();
     let conn = req.app().db_read()?;
     let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
     let owners = Team::owning(&krate, &conn)?
@@ -35,7 +35,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 crate_name = req.param("crate_id").unwrap();
     let conn = req.app().db_read()?;
     let krate: Crate = Crate::by_name(crate_name).first(&*conn)?;
     let owners = User::owning(&krate, &conn)?
@@ -81,7 +81,7 @@ fn parse_owners_request(req: &mut dyn RequestExt) -> AppResult<Vec<String>> {
 }
 
 fn modify_owners(req: &mut dyn RequestExt, add: bool) -> EndpointResult {
-    let crate_name = &req.params()["crate_id"];
+    let crate_name = req.param("crate_id").unwrap();
 
     let auth = AuthCheck::default()
         .with_endpoint_scope(EndpointScope::ChangeOwners)
@@ -90,7 +90,7 @@ fn modify_owners(req: &mut dyn RequestExt, add: bool) -> EndpointResult {
 
     let logins = parse_owners_request(req)?;
     let app = req.app();
-    let crate_name = &req.params()["crate_id"];
+    let crate_name = req.param("crate_id").unwrap();
 
     let conn = app.db_write()?;
     let user = auth.user();
diff --git a/src/controllers/metrics.rs b/src/controllers/metrics.rs
@@ -24,7 +24,7 @@ pub fn prometheus(req: &mut dyn RequestExt) -> EndpointResult {
         return Err(Box::new(MetricsDisabled));
     }
 
-    let metrics = match req.params()["kind"].as_str() {
+    let metrics = match req.param("kind").unwrap() {
         "service" => app.service_metrics.gather(&*app.db_read()?)?,
         "instance" => app.instance_metrics.gather(app)?,
         _ => return Err(not_found()),
diff --git a/src/controllers/team.rs b/src/controllers/team.rs
@@ -8,7 +8,7 @@ use crate::views::EncodableTeam;
 pub fn show_team(req: &mut dyn RequestExt) -> EndpointResult {
     use self::teams::dsl::{login, teams};
 
-    let name = &req.params()["team_id"];
+    let name = req.param("team_id").unwrap();
     let conn = req.app().db_read()?;
     let team: Team = teams.filter(login.eq(name)).first(&*conn)?;
 
diff --git a/src/controllers/token.rs b/src/controllers/token.rs
@@ -87,7 +87,9 @@ pub fn new(req: &mut dyn RequestExt) -> EndpointResult {
 
 /// Handles the `DELETE /me/tokens/:id` route.
 pub fn revoke(req: &mut dyn RequestExt) -> EndpointResult {
-    let id = req.params()["id"]
+    let id = req
+        .param("id")
+        .unwrap()
         .parse::<i32>()
         .map_err(|e| bad_request(&format!("invalid token id: {e:?}")))?;
 
diff --git a/src/controllers/user/me.rs b/src/controllers/user/me.rs
@@ -102,14 +102,14 @@ pub fn update_user(req: &mut dyn RequestExt) -> EndpointResult {
     let mut body = String::new();
     req.body().read_to_string(&mut body)?;
 
-    let param_user_id = &req.params()["user_id"];
+    let param_user_id = req.param("user_id").unwrap();
 
     let state = req.app();
     let conn = state.db_write()?;
     let user = auth.user();
 
     // need to check if current user matches user to be updated
-    if &user.id.to_string() != param_user_id {
+    if user.id.to_string() != param_user_id {
         return Err(bad_request("current user does not match requested user"));
     }
 
@@ -169,7 +169,7 @@ pub fn confirm_user_email(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::update;
 
     let conn = req.app().db_write()?;
-    let req_token = &req.params()["email_token"];
+    let req_token = req.param("email_token").unwrap();
 
     let updated_rows = update(emails::table.filter(emails::token.eq(req_token)))
         .set(emails::verified.eq(true))
@@ -187,7 +187,9 @@ pub fn regenerate_token_and_send(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::dsl::sql;
     use diesel::update;
 
-    let param_user_id = req.params()["user_id"]
+    let param_user_id = req
+        .param("user_id")
+        .unwrap()
         .parse::<i32>()
         .map_err(|err| err.chain(bad_request("invalid user_id")))?;
 
diff --git a/src/controllers/user/other.rs b/src/controllers/user/other.rs
@@ -9,7 +9,7 @@ use crate::views::EncodablePublicUser;
 pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
     use self::users::dsl::{gh_login, id, users};
 
-    let name = lower(&req.params()["user_id"]);
+    let name = lower(req.param("user_id").unwrap());
     let conn = req.app().db_read_prefer_primary()?;
     let user: User = users
         .filter(lower(gh_login).eq(name))
@@ -23,7 +23,9 @@ pub fn show(req: &mut dyn RequestExt) -> EndpointResult {
 pub fn stats(req: &mut dyn RequestExt) -> EndpointResult {
     use diesel::dsl::sum;
 
-    let user_id = &req.params()["user_id"]
+    let user_id = req
+        .param("user_id")
+        .unwrap()
         .parse::<i32>()
         .map_err(|err| err.chain(bad_request("invalid user_id")))?;
     let conn = req.app().db_read_prefer_primary()?;
diff --git a/src/controllers/util.rs b/src/controllers/util.rs
@@ -1,5 +1,6 @@
 use super::prelude::*;
 use crate::util::errors::{forbidden, internal, AppError, AppResult};
+use conduit_router::RequestParams;
 
 /// The Origin header (https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin)
 /// is sent with CORS requests and POST requests, and indicates where the request comes from.
@@ -22,3 +23,13 @@ pub fn verify_origin(req: &dyn RequestExt) -> AppResult<()> {
     }
     Ok(())
 }
+
+pub trait RequestParamExt<'a> {
+    fn param(self, key: &str) -> Option<&'a str>;
+}
+
+impl<'a> RequestParamExt<'a> for &'a (dyn RequestExt + 'a) {
+    fn param(self, key: &str) -> Option<&'a str> {
+        self.params().find(key)
+    }
+}
diff --git a/src/controllers/version.rs b/src/controllers/version.rs
@@ -25,11 +25,11 @@ fn extract_crate_name_and_semver(req: &dyn RequestExt) -> AppResult<(&str, &str)
 }
 
 fn extract_crate_name(req: &dyn RequestExt) -> &str {
-    &req.params()["crate_id"]
+    req.param("crate_id").unwrap()
 }
 
 fn extract_semver(req: &dyn RequestExt) -> AppResult<&str> {
-    let semver = &req.params()["version"];
+    let semver = req.param("version").unwrap();
     if semver::Version::parse(semver).is_err() {
         return Err(cargo_err(&format_args!("invalid semver: {semver}")));
     };
diff --git a/src/controllers/version/deprecated.rs b/src/controllers/version/deprecated.rs
@@ -52,7 +52,7 @@ pub fn index(req: &mut dyn RequestExt) -> EndpointResult {
 /// The frontend doesn't appear to hit this endpoint. Instead, the version information appears to
 /// be returned by `krate::show`.
 pub fn show_by_id(req: &mut dyn RequestExt) -> EndpointResult {
-    let id = &req.params()["version_id"];
+    let id = req.param("version_id").unwrap();
     let id = id.parse().unwrap_or(0);
     let conn = req.app().db_read()?;
     let (version, krate, published_by): (Version, Crate, Option<User>) = versions::table
diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs
@@ -16,8 +16,8 @@ use chrono::{Duration, NaiveDate, Utc};
 pub fn download(req: &mut dyn RequestExt) -> EndpointResult {
     let app = req.app();
 
-    let mut crate_name = req.params()["crate_id"].clone();
-    let version = req.params()["version"].as_str();
+    let mut crate_name = req.param("crate_id").unwrap().to_string();
+    let version = req.param("version").unwrap();
 
     let mut log_metadata = None;
 

Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ pub fn prometheus(req: &mut dyn RequestExt) -> EndpointResult {`
`24`	`24`	`return Err(Box::new(MetricsDisabled));`
`25`	`25`	`}`
`26`	`26`
`27`		`- let metrics = match req.params()["kind"].as_str() {`
	`27`	`+ let metrics = match req.param("kind").unwrap() {`
`28`	`28`	`"service" => app.service_metrics.gather(&*app.db_read()?)?,`
`29`	`29`	`"instance" => app.instance_metrics.gather(app)?,`
`30`	`30`	`_ => return Err(not_found()),`