Fix download counting (#309)

rustfmt.toml

@@ -0,0 +1,2 @@
+edition = "2018"
+max_width = 80

sqlx-data.json

@@ -151,18 +151,6 @@
       "nullable": []
     }
   },
-  "0483c9cf29bccba550dae1c602db928b83b77bca3007f0bba67f297797c8ceef": {
-    "query": "UPDATE mods\n                 SET downloads = downloads + 1\n                 WHERE (id = $1)",
-    "describe": {
-      "columns": [],
-      "parameters": {
-        "Left": [
-          "Int8"
-        ]
-      },
-      "nullable": []
-    }
-  },
   "04dcb2565608e296502694efc0c59bc77c41175ef65c830f2fef745773f18c86": {
     "query": "\n                        UPDATE mods\n                        SET moderation_message_body = NULL\n                        WHERE (id = $1)\n                        ",
     "describe": {
@@ -863,6 +851,27 @@
       "nullable": []
     }
   },
+  "331041c6a4f27f4a6ac2873332074c0127e7368c8ab803843760530d29aaef08": {
+    "query": "SELECT id FROM versions\n         WHERE (version_number = $1 AND mod_id = $2)",
+    "describe": {
+      "columns": [
+        {
+          "ordinal": 0,
+          "name": "id",
+          "type_info": "Int8"
+        }
+      ],
+      "parameters": {
+        "Left": [
+          "Text",
+          "Int8"
+        ]
+      },
+      "nullable": [
+        false
+      ]
+    }
+  },
   "33fc96ac71cfa382991cfb153e89da1e9f43ebf5367c28b30c336b758222307b": {
     "query": "\n            DELETE FROM loaders_versions\n            WHERE loaders_versions.version_id = $1\n            ",
     "describe": {
@@ -1667,26 +1676,6 @@
       "nullable": []
     }
   },
-  "59fd3b8da460fd1d81f3a1756fec609b05ce5d9eab035aa940d77753a341b599": {
-    "query": "SELECT mod_id FROM versions\n                 WHERE (id = $1)",
-    "describe": {
-      "columns": [
-        {
-          "ordinal": 0,
-          "name": "mod_id",
-          "type_info": "Int8"
-        }
-      ],
-      "parameters": {
-        "Left": [
-          "Int8"
-        ]
-      },
-      "nullable": [
-        false
-      ]
-    }
-  },
   "5a03c653f1ff3339a01422ee4267a66157e6da9a51cc7d9beb0f87d59c3a444c": {
     "query": "\n            SELECT d.dependent_id, d.dependency_id, d.mod_dependency_id\n            FROM versions v\n            INNER JOIN dependencies d ON d.dependent_id = v.id\n            WHERE v.mod_id = $1\n            ",
     "describe": {
@@ -3216,6 +3205,18 @@
       "nullable": []
     }
   },
+  "9bb993b9b743c3e6d3c8a9e4753983239b056cb9d4ec26e5fc8c6d4a5122e512": {
+    "query": "UPDATE mods\n             SET downloads = downloads + 1\n             WHERE (id = $1)",
+    "describe": {
+      "columns": [],
+      "parameters": {
+        "Left": [
+          "Int8"
+        ]
+      },
+      "nullable": []
+    }
+  },
   "9ceca63fb11f35f09f77bb9db175a1ac74dfcc2200c8134866922742fbbedea3": {
     "query": "\n            UPDATE dependencies\n            SET dependency_id = $2\n            WHERE dependency_id = $1\n            ",
     "describe": {

src/routes/v1/versions.rs

@@ -1,11 +1,13 @@
 use crate::file_hosting::FileHost;
 use crate::models::ids::{ProjectId, UserId, VersionId};
-use crate::models::projects::{Dependency, GameVersion, Loader, Version, VersionFile, VersionType};
+use crate::models::projects::{
+    Dependency, GameVersion, Loader, Version, VersionFile, VersionType,
+};
 use crate::models::teams::Permissions;
 use crate::routes::versions::{VersionIds, VersionListFilters};
 use crate::routes::ApiError;
 use crate::util::auth::get_user_from_headers;
-use crate::{database, models, Pepper};
+use crate::{database, models};
 use actix_web::{delete, get, web, HttpRequest, HttpResponse};
 use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
@@ -60,7 +62,9 @@ pub async fn version_list(
 ) -> Result<HttpResponse, ApiError> {
     let string = info.into_inner().0;
 
-    let result = database::models::Project::get_from_slug_or_project_id(string, &**pool).await?;
+    let result =
+        database::models::Project::get_from_slug_or_project_id(string, &**pool)
+            .await?;
 
     if let Some(project) = result {
         let id = project.id;
@@ -79,7 +83,9 @@ pub async fn version_list(
         )
         .await?;
 
-        let mut versions = database::models::Version::get_many_full(version_ids, &**pool).await?;
+        let mut versions =
+            database::models::Version::get_many_full(version_ids, &**pool)
+                .await?;
 
         let mut response = versions
             .iter()
@@ -97,11 +103,19 @@ pub async fn version_list(
         versions.sort_by(|a, b| b.date_published.cmp(&a.date_published));
 
         // Attempt to populate versions with "auto featured" versions
-        if response.is_empty() && !versions.is_empty() && filters.featured.unwrap_or(false) {
+        if response.is_empty()
-            let loaders = database::models::categories::Loader::list(&**pool).await?;
+            && !versions.is_empty()
+            && filters.featured.unwrap_or(false)
+        {
+            let loaders =
+                database::models::categories::Loader::list(&**pool).await?;
             let game_versions =
-                database::models::categories::GameVersion::list_filter(None, Some(true), &**pool)
+                database::models::categories::GameVersion::list_filter(
-                    .await?;
+                    None,
+                    Some(true),
+                    &**pool,
+                )
+                .await?;
 
             let mut joined_filters = Vec::new();
             for game_version in &game_versions {
@@ -117,14 +131,18 @@ pub async fn version_list(
                         version.game_versions.contains(&filter.0.version)
                             && version.loaders.contains(&filter.1.loader)
                     })
-                    .map(|version| response.push(convert_to_legacy(Version::from(version.clone()))))
+                    .map(|version| {
+                        response.push(convert_to_legacy(Version::from(
+                            version.clone(),
+                        )))
+                    })
                     .unwrap_or(());
             });
 
             if response.is_empty() {
-                versions
+                versions.into_iter().for_each(|version| {
-                    .into_iter()
+                    response.push(convert_to_legacy(Version::from(version)))
-                    .for_each(|version| response.push(convert_to_legacy(Version::from(version))));
+                });
             }
         }
 
@@ -142,11 +160,13 @@ pub async fn versions_get(
     ids: web::Query<VersionIds>,
     pool: web::Data<PgPool>,
 ) -> Result<HttpResponse, ApiError> {
-    let version_ids = serde_json::from_str::<Vec<models::ids::VersionId>>(&*ids.ids)?
+    let version_ids =
-        .into_iter()
+        serde_json::from_str::<Vec<models::ids::VersionId>>(&*ids.ids)?
-        .map(|x| x.into())
+            .into_iter()
-        .collect();
+            .map(|x| x.into())
-    let versions_data = database::models::Version::get_many_full(version_ids, &**pool).await?;
+            .collect();
+    let versions_data =
+        database::models::Version::get_many_full(version_ids, &**pool).await?;
 
     let mut versions = Vec::new();
 
@@ -163,7 +183,8 @@ pub async fn version_get(
     pool: web::Data<PgPool>,
 ) -> Result<HttpResponse, ApiError> {
     let id = info.into_inner().0;
-    let version_data = database::models::Version::get_full(id.into(), &**pool).await?;
+    let version_data =
+        database::models::Version::get_full(id.into(), &**pool).await?;
 
     if let Some(data) = version_data {
         Ok(HttpResponse::Ok().json(convert_to_legacy(Version::from(data))))
@@ -211,7 +232,8 @@ pub async fn get_version_from_hash(
         .await?;
 
         if let Some(data) = version_data {
-            Ok(HttpResponse::Ok().json(crate::models::projects::Version::from(data)))
+            Ok(HttpResponse::Ok()
+                .json(crate::models::projects::Version::from(data)))
         } else {
             Ok(HttpResponse::NotFound().body(""))
         }
@@ -287,25 +309,28 @@ pub async fn delete_file(
 
     if let Some(row) = result {
         if !user.role.is_mod() {
-            let team_member = database::models::TeamMember::get_from_user_id_version(
+            let team_member =
-                database::models::ids::VersionId(row.version_id),
+                database::models::TeamMember::get_from_user_id_version(
-                user.id.into(),
+                    database::models::ids::VersionId(row.version_id),
-                &**pool,
+                    user.id.into(),
-            )
+                    &**pool,
-            .await
-            .map_err(ApiError::DatabaseError)?
-            .ok_or_else(|| {
-                ApiError::CustomAuthenticationError(
-                    "You don't have permission to delete this file!".to_string(),
                 )
-            })?;
+                .await
+                .map_err(ApiError::DatabaseError)?
+                .ok_or_else(|| {
+                    ApiError::CustomAuthenticationError(
+                        "You don't have permission to delete this file!"
+                            .to_string(),
+                    )
+                })?;
 
             if !team_member
                 .permissions
                 .contains(Permissions::DELETE_VERSION)
             {
                 return Err(ApiError::CustomAuthenticationError(
-                    "You don't have permission to delete this file!".to_string(),
+                    "You don't have permission to delete this file!"
+                        .to_string(),
                 ));
             }
         }

src/routes/versions.rs

@@ -1,5 +1,6 @@
 use super::ApiError;
 use crate::database;
+use crate::database::models as db_models;
 use crate::models;
 use crate::models::projects::{Dependency, Version};
 use crate::models::teams::Permissions;
@@ -26,7 +27,9 @@ pub async fn version_list(
 ) -> Result<HttpResponse, ApiError> {
     let string = info.into_inner().0;
 
-    let result = database::models::Project::get_from_slug_or_project_id(string, &**pool).await?;
+    let result =
+        database::models::Project::get_from_slug_or_project_id(string, &**pool)
+            .await?;
 
     if let Some(project) = result {
         let id = project.id;
@@ -45,7 +48,9 @@ pub async fn version_list(
         )
         .await?;
 
-        let mut versions = database::models::Version::get_many_full(version_ids, &**pool).await?;
+        let mut versions =
+            database::models::Version::get_many_full(version_ids, &**pool)
+                .await?;
 
         let mut response = versions
             .iter()
@@ -62,10 +67,17 @@ pub async fn version_list(
         versions.sort_by(|a, b| b.date_published.cmp(&a.date_published));
 
         // Attempt to populate versions with "auto featured" versions
-        if response.is_empty() && !versions.is_empty() && filters.featured.unwrap_or(false) {
+        if response.is_empty()
+            && !versions.is_empty()
+            && filters.featured.unwrap_or(false)
+        {
             let (loaders, game_versions) = futures::join!(
                 database::models::categories::Loader::list(&**pool),
-                database::models::categories::GameVersion::list_filter(None, Some(true), &**pool)
+                database::models::categories::GameVersion::list_filter(
+                    None,
+                    Some(true),
+                    &**pool
+                )
             );
 
             let (loaders, game_versions) = (loaders?, game_versions?);
@@ -84,7 +96,9 @@ pub async fn version_list(
                         version.game_versions.contains(&filter.0.version)
                             && version.loaders.contains(&filter.1.loader)
                     })
-                    .map(|version| response.push(Version::from(version.clone())))
+                    .map(|version| {
+                        response.push(Version::from(version.clone()))
+                    })
                     .unwrap_or(());
             });
 
@@ -114,11 +128,13 @@ pub async fn versions_get(
     web::Query(ids): web::Query<VersionIds>,
     pool: web::Data<PgPool>,
 ) -> Result<HttpResponse, ApiError> {
-    let version_ids = serde_json::from_str::<Vec<models::ids::VersionId>>(&*ids.ids)?
+    let version_ids =
-        .into_iter()
+        serde_json::from_str::<Vec<models::ids::VersionId>>(&*ids.ids)?
-        .map(|x| x.into())
+            .into_iter()
-        .collect();
+            .map(|x| x.into())
-    let versions_data = database::models::Version::get_many_full(version_ids, &**pool).await?;
+            .collect();
+    let versions_data =
+        database::models::Version::get_many_full(version_ids, &**pool).await?;
 
     let versions = versions_data
         .into_iter()
@@ -133,7 +149,8 @@ pub async fn version_get(
     pool: web::Data<PgPool>,
 ) -> Result<HttpResponse, ApiError> {
     let id = info.into_inner().0;
-    let version_data = database::models::Version::get_full(id.into(), &**pool).await?;
+    let version_data =
+        database::models::Version::get_full(id.into(), &**pool).await?;
 
     if let Some(data) = version_data {
         Ok(HttpResponse::Ok().json(models::projects::Version::from(data)))
@@ -170,9 +187,9 @@ pub async fn version_edit(
 ) -> Result<HttpResponse, ApiError> {
     let user = get_user_from_headers(req.headers(), &**pool).await?;
 
-    new_version
+    new_version.validate().map_err(|err| {
-        .validate()
+        ApiError::ValidationError(validation_errors_to_string(err, None))
-        .map_err(|err| ApiError::ValidationError(validation_errors_to_string(err, None)))?;
+    })?;
 
     let version_id = info.into_inner().0;
     let id = version_id.into();
@@ -180,12 +197,13 @@ pub async fn version_edit(
     let result = database::models::Version::get_full(id, &**pool).await?;
 
     if let Some(version_item) = result {
-        let team_member = database::models::TeamMember::get_from_user_id_version(
+        let team_member =
-            version_item.id,
+            database::models::TeamMember::get_from_user_id_version(
-            user.id.into(),
+                version_item.id,
-            &**pool,
+                user.id.into(),
-        )
+                &**pool,
-        .await?;
+            )
+            .await?;
         let permissions;
 
         if let Some(member) = team_member {
@@ -199,7 +217,8 @@ pub async fn version_edit(
         if let Some(perms) = permissions {
             if !perms.contains(Permissions::UPLOAD_VERSION) {
                 return Err(ApiError::CustomAuthenticationError(
-                    "You do not have the permissions to edit this version!".to_string(),
+                    "You do not have the permissions to edit this version!"
+                        .to_string(),
                 ));
             }
 
@@ -267,7 +286,9 @@ pub async fn version_edit(
                     .collect::<Vec<database::models::version_item::DependencyBuilder>>();
 
                 for dependency in builders {
-                    dependency.insert(version_item.id, &mut transaction).await?;
+                    dependency
+                        .insert(version_item.id, &mut transaction)
+                        .await?;
                 }
             }
 
@@ -282,16 +303,18 @@ pub async fn version_edit(
                 .await?;
 
                 for game_version in game_versions {
-                    let game_version_id = database::models::categories::GameVersion::get_id(
+                    let game_version_id =
-                        &game_version.0,
+                        database::models::categories::GameVersion::get_id(
-                        &mut *transaction,
+                            &game_version.0,
-                    )
+                            &mut *transaction,
-                    .await?
-                    .ok_or_else(|| {
-                        ApiError::InvalidInputError(
-                            "No database entry for game version provided.".to_string(),
                         )
-                    })?;
+                        .await?
+                        .ok_or_else(|| {
+                            ApiError::InvalidInputError(
+                                "No database entry for game version provided."
+                                    .to_string(),
+                            )
+                        })?;
 
                     sqlx::query!(
                         "
@@ -318,13 +341,17 @@ pub async fn version_edit(
 
                 for loader in loaders {
                     let loader_id =
-                        database::models::categories::Loader::get_id(&loader.0, &mut *transaction)
+                        database::models::categories::Loader::get_id(
-                            .await?
+                            &loader.0,
-                            .ok_or_else(|| {
+                            &mut *transaction,
-                                ApiError::InvalidInputError(
+                        )
-                                    "No database entry for loader provided.".to_string(),
+                        .await?
-                                )
+                        .ok_or_else(|| {
-                            })?;
+                            ApiError::InvalidInputError(
+                                "No database entry for loader provided."
+                                    .to_string(),
+                            )
+                        })?;
 
                     sqlx::query!(
                         "
@@ -422,41 +449,47 @@ pub async fn version_edit(
 }
 
 // This is an internal route, cannot be used without key
-#[patch("{version_id}/_count-download", guard = "admin_key_guard")]
+#[patch(
+    "{project_id}/{version_name}/_count-download",
+    guard = "admin_key_guard"
+)]
 pub async fn version_count_patch(
-    info: web::Path<(models::ids::VersionId,)>,
+    info: web::Path<(models::ids::ProjectId, String)>,
     pool: web::Data<PgPool>,
 ) -> Result<HttpResponse, ApiError> {
-    let version = info.into_inner().0;
+    let (project, version_name) = info.into_inner();
-    let version = database::models::ids::VersionId::from(version);
+    let project = db_models::ids::ProjectId::from(project);
+
+    let version = sqlx::query!(
+        "SELECT id FROM versions
+         WHERE (version_number = $1 AND mod_id = $2)",
+        version_name,
+        project as db_models::ids::ProjectId
+    )
+    .fetch_optional(pool.as_ref())
+    .await?;
+    let version = match version {
+        Some(version) => db_models::ids::VersionId(version.id),
+        _ => {
+            return Ok(HttpResponse::NotFound().body("Could not find version!"))
+        }
+    };
 
     futures::future::try_join(
         sqlx::query!(
             "UPDATE versions
              SET downloads = downloads + 1
              WHERE (id = $1)",
-            version as database::models::ids::VersionId
+            version as db_models::ids::VersionId
+        )
+        .execute(pool.as_ref()),
+        sqlx::query!(
+            "UPDATE mods
+             SET downloads = downloads + 1
+             WHERE (id = $1)",
+            project as db_models::ids::ProjectId
         )
         .execute(pool.as_ref()),
-        async {
-            let project_id = sqlx::query!(
-                "SELECT mod_id FROM versions
-                 WHERE (id = $1)",
-                version as database::models::ids::VersionId
-            )
-            .fetch_one(pool.as_ref())
-            .await?
-            .mod_id;
-
-            sqlx::query!(
-                "UPDATE mods
-                 SET downloads = downloads + 1
-                 WHERE (id = $1)",
-                project_id
-            )
-            .execute(pool.as_ref())
-            .await
-        },
     )
     .await
     .map_err(ApiError::SqlxDatabaseError)?;
@@ -492,14 +525,17 @@ pub async fn version_delete(
             .contains(Permissions::DELETE_VERSION)
         {
             return Err(ApiError::CustomAuthenticationError(
-                "You do not have permission to delete versions in this team".to_string(),
+                "You do not have permission to delete versions in this team"
+                    .to_string(),
             ));
         }
     }
 
     let mut transaction = pool.begin().await?;
 
-    let result = database::models::Version::remove_full(id.into(), &mut transaction).await?;
+    let result =
+        database::models::Version::remove_full(id.into(), &mut transaction)
+            .await?;
 
     transaction.commit().await?;
 

src/util/guards.rs

@@ -2,8 +2,9 @@ use actix_web::guard::GuardContext;
 
 pub const ADMIN_KEY_HEADER: &str = "Modrinth-Admin";
 pub fn admin_key_guard(ctx: &GuardContext) -> bool {
-    let admin_key = std::env::var("LABRINTH_ADMIN_KEY")
+    let admin_key = std::env::var("LABRINTH_ADMIN_KEY").expect(
-        .expect("No admin key provided, this should have been caught by check_env_vars");
+        "No admin key provided, this should have been caught by check_env_vars",
+    );
 
     ctx.head()
         .headers()