Refactor queries to use the more semantic postgres::Client methods

Author: Nemo157

src/bin/cratesfyi.rs

@@ -372,9 +372,9 @@ impl BuildSubcommand {
                         .get()
                         .context("failed to get a database connection")?;
                     let res =
-                        conn.query("SELECT * FROM config WHERE name = 'rustc_version';", &[])?;
+                        conn.query_one("SELECT COUNT(*) > 1 FROM config WHERE name = 'rustc_version';", &[])?;
 
-                    if !res.is_empty() {
+                    if res.get(0) {
                         println!("update-toolchain was already called in the past, exiting");
                         return Ok(());
                     }

src/build_queue.rs

@@ -35,27 +35,27 @@ impl BuildQueue {
     }
 
     pub(crate) fn pending_count(&self) -> Result<usize> {
-        let res = self.db.get()?.query(
+        let res = self.db.get()?.query_one(
             "SELECT COUNT(*) FROM queue WHERE attempt < $1;",
             &[&self.max_attempts],
         )?;
-        Ok(res[0].get::<_, i64>(0) as usize)
+        Ok(res.get::<_, i64>(0) as usize)
     }
 
     pub(crate) fn prioritized_count(&self) -> Result<usize> {
-        let res = self.db.get()?.query(
+        let res = self.db.get()?.query_one(
             "SELECT COUNT(*) FROM queue WHERE attempt < $1 AND priority <= 0;",
             &[&self.max_attempts],
         )?;
-        Ok(res[0].get::<_, i64>(0) as usize)
+        Ok(res.get::<_, i64>(0) as usize)
     }
 
     pub(crate) fn failed_count(&self) -> Result<usize> {
-        let res = self.db.get()?.query(
+        let res = self.db.get()?.query_one(
             "SELECT COUNT(*) FROM queue WHERE attempt >= $1;",
             &[&self.max_attempts],
         )?;
-        Ok(res[0].get::<_, i64>(0) as usize)
+        Ok(res.get::<_, i64>(0) as usize)
     }
 
     pub(crate) fn queued_crates(&self) -> Result<Vec<QueuedCrate>> {
@@ -98,11 +98,11 @@ impl BuildQueue {
             }
             Err(e) => {
                 // Increase attempt count
-                let rows = conn.query(
+                let rows = conn.query_one(
                     "UPDATE queue SET attempt = attempt + 1 WHERE id = $1 RETURNING attempt;",
                     &[&to_process.id],
                 )?;
-                let attempt: i32 = rows[0].get(0);
+                let attempt: i32 = rows.get(0);
 
                 if attempt >= self.max_attempts {
                     crate::web::metrics::FAILED_BUILDS.inc();

src/db/add_package.rs

@@ -137,7 +137,7 @@ pub(crate) fn add_build_into_database(
     res: &BuildResult,
 ) -> Result<i32> {
     debug!("Adding build into database");
-    let rows = conn.query(
+    let row = conn.query_one(
         "INSERT INTO builds (rid, rustc_version,
                                                     cratesfyi_version,
                                                     build_status, output)
@@ -151,19 +151,20 @@ pub(crate) fn add_build_into_database(
             &res.build_log,
         ],
     )?;
-    Ok(rows[0].get(0))
+    Ok(row.get(0))
 }
 
 fn initialize_package_in_database(conn: &mut Client, pkg: &MetadataPackage) -> Result<i32> {
-    let mut rows = conn.query("SELECT id FROM crates WHERE name = $1", &[&pkg.name])?;
     // insert crate into database if it is not exists
-    if rows.is_empty() {
-        rows = conn.query(
-            "INSERT INTO crates (name) VALUES ($1) RETURNING id",
+    let row = conn.query_opt("SELECT id FROM crates WHERE name = $1", &[&pkg.name])
+        .transpose()
+        .unwrap_or_else(|| conn.query_one(
+            "INSERT INTO crates (name)
+             VALUES ($1)
+             RETURNING id",
             &[&pkg.name],
-        )?;
-    }
-    Ok(rows[0].get(0))
+        ))?;
+    Ok(row.get(0))
 }
 
 /// Convert dependencies into Vec<(String, String, String)>
@@ -257,23 +258,23 @@ fn add_keywords_into_database(
     for keyword in &pkg.keywords {
         let slug = slugify(&keyword);
         let keyword_id: i32 = {
-            let rows = conn.query("SELECT id FROM keywords WHERE slug = $1", &[&slug])?;
-            if !rows.is_empty() {
-                rows[0].get(0)
-            } else {
-                conn.query(
-                    "INSERT INTO keywords (name, slug) VALUES ($1, $2) RETURNING id",
+            conn.query_opt("SELECT id FROM keywords WHERE slug = $1", &[&slug])
+                .transpose()
+                .unwrap_or_else(|| conn.query_one(
+                    "INSERT INTO keywords (name, slug)
+                     VALUES ($1, $2)
+                     RETURNING id",
                     &[&keyword, &slug],
-                )?[0]
-                    .get(0)
-            }
+                ))?.get(0)
         };
 
-        // add releationship
-        let _ = conn.query(
-            "INSERT INTO keyword_rels (rid, kid) VALUES ($1, $2)",
+        // add relationship
+        conn.execute(
+            "INSERT INTO keyword_rels (rid, kid)
+             VALUES ($1, $2)
+             ON CONFLICT DO NOTHING",
             &[&release_id, &keyword_id],
-        );
+        )?;
     }
 
     Ok(())
@@ -301,24 +302,31 @@ fn add_authors_into_database(
             let slug = slugify(&author);
 
             let author_id: i32 = {
-                let rows = conn.query("SELECT id FROM authors WHERE slug = $1", &[&slug])?;
-                if !rows.is_empty() {
-                    rows[0].get(0)
-                } else {
-                    conn.query(
-                        "INSERT INTO authors (name, email, slug) VALUES ($1, $2, $3)
-                                     RETURNING id",
-                        &[&author, &email, &slug],
-                    )?[0]
-                        .get(0)
-                }
+                // TODO: If an author uses different email addresses per crate, or multiple authors
+                // have the same slugified name, this will flip-flop their name and email address
+                // on each crate publish.
+                // 
+                // But, at least doing an upsert will result in their email/exact name rendering
+                // being updated if they change it in a new release.
+                conn.query_one(
+                    "INSERT INTO authors (name, email, slug)
+                     VALUES ($1, $2, $3)
+                     ON CONFLICT (slug) DO UPDATE
+                       SET
+                         name = $1,
+                         email = $2
+                     RETURNING id",
+                    &[&author, &email, &slug],
+                )?.get(0)
             };
 
             // add relationship
-            let _ = conn.query(
-                "INSERT INTO author_rels (rid, aid) VALUES ($1, $2)",
+            conn.execute(
+                "INSERT INTO author_rels (rid, aid)
+                 VALUES ($1, $2)
+                 ON CONFLICT DO NOTHING",
                 &[&release_id, &author_id],
-            );
+            )?;
         }
     }
 
@@ -331,7 +339,12 @@ pub fn update_crate_data_in_database(
     registry_data: &CrateData,
 ) -> Result<()> {
     info!("Updating crate data for {}", name);
-    let crate_id = conn.query("SELECT id FROM crates WHERE crates.name = $1", &[&name])?[0].get(0);
+    let crate_id = conn.query_one(
+        "SELECT id
+         FROM crates
+         WHERE crates.name = $1",
+         &[&name],
+     )?.get(0);
 
     update_owners_in_database(conn, &registry_data.owners, crate_id)?;
 
@@ -362,7 +375,7 @@ fn update_owners_in_database(
         // Update any existing owner data since it is mutable and could have changed since last
         // time we pulled it
         let owner_id: i32 = {
-            conn.query(
+            conn.query_one(
                 "
                     INSERT INTO owners (login, avatar, name, email)
                     VALUES ($1, $2, $3, $4)
@@ -374,12 +387,11 @@ fn update_owners_in_database(
                     RETURNING id
                 ",
                 &[&owner.login, &owner.avatar, &owner.name, &owner.email],
-            )?[0]
-                .get(0)
+            )?.get(0)
         };
 
         // add relationship
-        conn.query(
+        conn.execute(
             "INSERT INTO owner_rels (cid, oid) VALUES ($1, $2) ON CONFLICT DO NOTHING",
             &[&crate_id, &owner_id],
         )?;
@@ -391,7 +403,7 @@ fn update_owners_in_database(
     for login in to_remove {
         debug!("Removing owner relationship {}", login);
         // remove relationship
-        conn.query(
+        conn.execute(
             "
                 DELETE FROM owner_rels
                 USING owners
@@ -417,7 +429,7 @@ where
     ON CONFLICT DO NOTHING;";
     let prepared = conn.prepare(sql)?;
     for alg in algorithms {
-        conn.query(&prepared, &[&release_id, &(alg as i32)])?;
+        conn.execute(&prepared, &[&release_id, &(alg as i32)])?;
     }
     Ok(())
 }

src/db/blacklist.rs

@@ -12,11 +12,11 @@ enum BlacklistError {
 
 /// Returns whether the given name is blacklisted.
 pub fn is_blacklisted(conn: &mut Client, name: &str) -> Result<bool, Error> {
-    let rows = conn.query(
+    let row = conn.query_one(
         "SELECT COUNT(*) FROM blacklisted_crates WHERE crate_name = $1;",
         &[&name],
     )?;
-    let count: i64 = rows[0].get(0);
+    let count: i64 = row.get(0);
 
     Ok(count != 0)
 }

src/db/delete.rs

@@ -39,8 +39,8 @@ pub fn delete_version(
 }
 
 fn get_id(conn: &mut Client, name: &str) -> Result<i32, Error> {
-    let crate_id_res = conn.query("SELECT id FROM crates WHERE name = $1", &[&name])?;
-    if let Some(row) = crate_id_res.into_iter().next() {
+    let row = conn.query_opt("SELECT id FROM crates WHERE name = $1", &[&name])?;
+    if let Some(row) = row {
         Ok(row.get("id"))
     } else {
         Err(CrateDeletionError::MissingCrate(name.into()).into())
@@ -123,15 +123,15 @@ mod tests {
     use postgres::Client;
 
     fn crate_exists(conn: &mut Client, name: &str) -> Result<bool, Error> {
-        Ok(!conn
-            .query("SELECT * FROM crates WHERE name = $1;", &[&name])?
-            .is_empty())
+        Ok(conn
+            .query_one("SELECT COUNT(*) > 0 FROM crates WHERE name = $1;", &[&name])?
+            .get(0))
     }
 
     fn release_exists(conn: &mut Client, id: i32) -> Result<bool, Error> {
-        Ok(!conn
-            .query("SELECT * FROM releases WHERE id = $1;", &[&id])?
-            .is_empty())
+        Ok(conn
+            .query_one("SELECT COUNT(*) > 0 FROM releases WHERE id = $1;", &[&id])?
+            .get(0))
     }
 
     #[test]
@@ -160,7 +160,7 @@ mod tests {
 
             let pkg1_id = &db
                 .conn()
-                .query("SELECT id FROM crates WHERE name = 'package-1';", &[])?[0]
+                .query_one("SELECT id FROM crates WHERE name = 'package-1';", &[])?
                 .get("id");
 
             delete_crate_from_database(&mut db.conn(), "package-1", *pkg1_id)?;
@@ -209,10 +209,7 @@ mod tests {
             assert!(release_exists(&mut db.conn(), v2)?);
             let crate_id = db
                 .conn()
-                .query("SELECT crate_id FROM releases WHERE id = $1", &[&v1])?
-                .into_iter()
-                .next()
-                .unwrap()
+                .query_one("SELECT crate_id FROM releases WHERE id = $1", &[&v1])?
                 .get(0);
             assert_eq!(
                 authors(&mut db.conn(), crate_id)?,

src/docbuilder/limits.rs

@@ -28,12 +28,11 @@ impl Limits {
     pub(crate) fn for_crate(conn: &mut Client, name: &str) -> Result<Self> {
         let mut limits = Self::default();
 
-        let res = conn.query(
+        let res = conn.query_opt(
             "SELECT * FROM sandbox_overrides WHERE crate_name = $1;",
             &[&name],
         )?;
-        if !res.is_empty() {
-            let row = &res[0];
+        if let Some(row) = res {
             if let Some(memory) = row.get::<_, Option<i64>>("max_memory_bytes") {
                 limits.memory = memory as usize;
             }
@@ -84,7 +83,7 @@ mod test {
             let hexponent = Limits::for_crate(&mut db.conn(), krate)?;
             assert_eq!(hexponent, Limits::default());
 
-            db.conn().query(
+            db.conn().execute(
                 "INSERT INTO sandbox_overrides (crate_name, max_targets) VALUES ($1, 15)",
                 &[&krate],
             )?;
@@ -106,7 +105,7 @@ mod test {
                 targets: 1,
                 ..Limits::default()
             };
-            db.conn().query(
+            db.conn().execute(
                 "INSERT INTO sandbox_overrides (crate_name, max_memory_bytes, timeout_seconds, max_targets)
                  VALUES ($1, $2, $3, $4)",
                 &[&krate, &(limits.memory as i64), &(limits.timeout.as_secs() as i32), &(limits.targets as i32)]

src/docbuilder/mod.rs

@@ -68,7 +68,7 @@ impl DocBuilder {
         debug!("Loading database cache");
 
         let mut conn = self.db.get()?;
-        for row in &mut conn.query(
+        for row in conn.query(
             "SELECT name, version FROM crates, releases \
              WHERE crates.id = releases.crate_id",
             &[],

src/docbuilder/rustwide_builder.rs

@@ -260,7 +260,7 @@ impl RustwideBuilder {
                 }
 
                 add_path_into_database(&self.storage, "", &dest)?;
-                conn.query(
+                conn.execute(
                     "INSERT INTO config (name, value) VALUES ('rustc_version', $1) \
                      ON CONFLICT (name) DO UPDATE SET value = $1;",
                     &[&Value::String(self.rustc_version.clone())],