Refactor querying crate owners by owner kind

Author: DSpeckhals

src/controllers/krate/search.rs

@@ -5,7 +5,7 @@ use diesel_full_text_search::*;
 
 use crate::controllers::helpers::Paginate;
 use crate::controllers::prelude::*;
-use crate::models::{Crate, CrateBadge, CrateVersions, OwnerKind, Version};
+use crate::models::{Crate, CrateBadge, CrateOwner, CrateVersions, OwnerKind, Version};
 use crate::schema::*;
 use crate::views::EncodableCrate;
 
@@ -118,21 +118,17 @@ pub fn search(req: &mut dyn Request) -> CargoResult<Response> {
     } else if let Some(user_id) = params.get("user_id").and_then(|s| s.parse::<i32>().ok()) {
         query = query.filter(
             crates::id.eq_any(
-                crate_owners::table
+                CrateOwner::by_owner_kind(OwnerKind::User)
                     .select(crate_owners::crate_id)
-                    .filter(crate_owners::owner_id.eq(user_id))
-                    .filter(crate_owners::deleted.eq(false))
-                    .filter(crate_owners::owner_kind.eq(OwnerKind::User as i32)),
+                    .filter(crate_owners::owner_id.eq(user_id)),
             ),
         );
     } else if let Some(team_id) = params.get("team_id").and_then(|s| s.parse::<i32>().ok()) {
         query = query.filter(
             crates::id.eq_any(
-                crate_owners::table
+                CrateOwner::by_owner_kind(OwnerKind::Team)
                     .select(crate_owners::crate_id)
-                    .filter(crate_owners::owner_id.eq(team_id))
-                    .filter(crate_owners::deleted.eq(false))
-                    .filter(crate_owners::owner_kind.eq(OwnerKind::Team as i32)),
+                    .filter(crate_owners::owner_id.eq(team_id)),
             ),
         );
     } else if params.get("following").is_some() {

src/controllers/user/me.rs

@@ -234,7 +234,6 @@ pub fn regenerate_token_and_send(req: &mut dyn Request) -> CargoResult<Response>
 /// Handles `PUT /me/email_notifications` route
 pub fn update_email_notifications(req: &mut dyn Request) -> CargoResult<Response> {
     use self::crate_owners::dsl::*;
-    // use diesel::update;
     use diesel::pg::upsert::excluded;
 
     #[derive(Deserialize)]
@@ -255,20 +254,26 @@ pub fn update_email_notifications(req: &mut dyn Request) -> CargoResult<Response
     let conn = req.db_conn()?;
 
     // Build inserts from existing crates beloning to the current user
-    let to_insert = CrateOwner::belonging_to(user)
+    let to_insert = CrateOwner::by_owner_kind(OwnerKind::User)
+        .filter(owner_id.eq(user.id))
         .select((crate_id, owner_id, owner_kind, email_notifications))
-        .filter(owner_kind.eq(OwnerKind::User as i32))
         .load(&*conn)?
         .into_iter()
-        .map(
+        // Remove records whose `email_notifications` will not change from their current value
+        .filter_map(
             |(c_id, o_id, o_kind, e_notifications): (i32, i32, i32, bool)| {
-                (
-                    crate_id.eq(c_id),
-                    owner_id.eq(o_id),
-                    owner_kind.eq(o_kind),
-                    email_notifications
-                        .eq(updates.get(&c_id).unwrap_or(&e_notifications).to_owned()),
-                )
+                let current_e_notifications = *updates.get(&c_id).unwrap_or(&e_notifications);
+
+                if e_notifications == current_e_notifications {
+                    None
+                } else {
+                    Some((
+                        crate_id.eq(c_id),
+                        owner_id.eq(o_id),
+                        owner_kind.eq(o_kind),
+                        email_notifications.eq(current_e_notifications),
+                    ))
+                }
             },
         )
         .collect::<Vec<_>>();

src/models/krate.rs

@@ -401,18 +401,17 @@ impl Crate {
     }
 
     pub fn owners(&self, conn: &PgConnection) -> CargoResult<Vec<Owner>> {
-        let base_query = CrateOwner::belonging_to(self).filter(crate_owners::deleted.eq(false));
-        let users = base_query
+        let users = CrateOwner::by_owner_kind(OwnerKind::User)
+            .filter(crate_owners::crate_id.eq(self.id))
             .inner_join(users::table)
             .select(users::all_columns)
-            .filter(crate_owners::owner_kind.eq(OwnerKind::User as i32))
             .load(conn)?
             .into_iter()
             .map(Owner::User);
-        let teams = base_query
+        let teams = CrateOwner::by_owner_kind(OwnerKind::Team)
+            .filter(crate_owners::crate_id.eq(self.id))
             .inner_join(teams::table)
             .select(teams::all_columns)
-            .filter(crate_owners::owner_kind.eq(OwnerKind::Team as i32))
             .load(conn)?
             .into_iter()
             .map(Owner::Team);

src/models/owner.rs

@@ -1,3 +1,4 @@
+use diesel::pg::Pg;
 use diesel::prelude::*;
 
 use crate::app::App;
@@ -22,6 +23,21 @@ pub struct CrateOwner {
     pub email_notifications: bool,
 }
 
+type BoxedQuery<'a> = crate_owners::BoxedQuery<'a, Pg, crate_owners::SqlType>;
+
+impl CrateOwner {
+    /// Returns a base crate owner query filtered by the owner kind argument. This query also
+    /// filters out deleted records.
+    pub fn by_owner_kind(kind: OwnerKind) -> BoxedQuery<'static> {
+        use self::crate_owners::dsl::*;
+
+        crate_owners
+            .filter(deleted.eq(false))
+            .filter(owner_kind.eq(kind as i32))
+            .into_boxed()
+    }
+}
+
 #[derive(Debug, Clone, Copy)]
 #[repr(u32)]
 pub enum OwnerKind {

src/models/user.rs

@@ -129,11 +129,10 @@ impl User {
     }
 
     pub fn owning(krate: &Crate, conn: &PgConnection) -> CargoResult<Vec<Owner>> {
-        let base_query = CrateOwner::belonging_to(krate).filter(crate_owners::deleted.eq(false));
-        let users = base_query
+        let users = CrateOwner::by_owner_kind(OwnerKind::User)
             .inner_join(users::table)
             .select(users::all_columns)
-            .filter(crate_owners::owner_kind.eq(OwnerKind::User as i32))
+            .filter(crate_owners::crate_id.eq(krate.id))
             .load(conn)?
             .into_iter()
             .map(Owner::User);