From a02aa7586b94fb4f1bc11f19e7688bf5f65b54f9 Mon Sep 17 00:00:00 2001 From: Geometrically <18202329+Geometrically@users.noreply.github.com> Date: Sun, 4 Feb 2024 20:19:46 -0700 Subject: [PATCH] Fix version files updates route (#876) * Fix version updates files route * run fmt + prep * actually work * update query perf * fix --- ...19e6c7b762bfbcc09d8ab2624b00113f71e77.json | 31 +++++ src/routes/v2/version_file.rs | 19 +-- src/routes/v3/version_file.rs | 109 +++++++----------- tests/common/api_v3/version.rs | 4 +- 4 files changed, 78 insertions(+), 85 deletions(-) create mode 100644 .sqlx/query-070174adf972b808aca7519168719e6c7b762bfbcc09d8ab2624b00113f71e77.json diff --git a/.sqlx/query-070174adf972b808aca7519168719e6c7b762bfbcc09d8ab2624b00113f71e77.json b/.sqlx/query-070174adf972b808aca7519168719e6c7b762bfbcc09d8ab2624b00113f71e77.json new file mode 100644 index 000000000..e9d49592e --- /dev/null +++ b/.sqlx/query-070174adf972b808aca7519168719e6c7b762bfbcc09d8ab2624b00113f71e77.json @@ -0,0 +1,31 @@ +{ + "db_name": "PostgreSQL", + "query": "\n SELECT v.id version_id, v.mod_id mod_id\n FROM versions v\n INNER JOIN version_fields vf ON vf.field_id = 3 AND v.id = vf.version_id\n INNER JOIN loader_field_enum_values lfev ON vf.enum_value = lfev.id AND (cardinality($2::varchar[]) = 0 OR lfev.value = ANY($2::varchar[]))\n INNER JOIN loaders_versions lv ON lv.version_id = v.id\n INNER JOIN loaders l on lv.loader_id = l.id AND (cardinality($3::varchar[]) = 0 OR l.loader = ANY($3::varchar[]))\n WHERE v.mod_id = ANY($1) AND (cardinality($4::varchar[]) = 0 OR v.version_type = ANY($4))\n ORDER BY v.date_published ASC\n ", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "version_id", + "type_info": "Int8" + }, + { + "ordinal": 1, + "name": "mod_id", + "type_info": "Int8" + } + ], + "parameters": { + "Left": [ + "Int8Array", + "VarcharArray", + "VarcharArray", + "VarcharArray" + ] + }, + "nullable": [ + false, + false + ] + }, + "hash": "070174adf972b808aca7519168719e6c7b762bfbcc09d8ab2624b00113f71e77" +} diff --git a/src/routes/v2/version_file.rs b/src/routes/v2/version_file.rs index 05224fe64..24e01270b 100644 --- a/src/routes/v2/version_file.rs +++ b/src/routes/v2/version_file.rs @@ -248,33 +248,22 @@ pub struct ManyUpdateData { #[post("update")] pub async fn update_files( - req: HttpRequest, pool: web::Data, redis: web::Data, update_data: web::Json, - session_queue: web::Data, ) -> Result { let update_data = update_data.into_inner(); - let mut loader_fields = HashMap::new(); - let mut game_versions = vec![]; - for gv in update_data.game_versions.into_iter().flatten() { - game_versions.push(serde_json::json!(gv.clone())); - } - if !game_versions.is_empty() { - loader_fields.insert("game_versions".to_string(), game_versions); - } let update_data = v3::version_file::ManyUpdateData { loaders: update_data.loaders.clone(), version_types: update_data.version_types.clone(), - loader_fields: Some(loader_fields), + game_versions: update_data.game_versions.clone(), algorithm: update_data.algorithm, hashes: update_data.hashes, }; - let response = - v3::version_file::update_files(req, pool, redis, web::Json(update_data), session_queue) - .await - .or_else(v2_reroute::flatten_404_error)?; + let response = v3::version_file::update_files(pool, redis, web::Json(update_data)) + .await + .or_else(v2_reroute::flatten_404_error)?; // Convert response to V2 format match v2_reroute::extract_ok_json::>(response).await { diff --git a/src/routes/v3/version_file.rs b/src/routes/v3/version_file.rs index 8400cfbd7..36e2f8814 100644 --- a/src/routes/v3/version_file.rs +++ b/src/routes/v3/version_file.rs @@ -9,6 +9,8 @@ use crate::models::teams::ProjectPermissions; use crate::queue::session::AuthQueue; use crate::{database, models}; use actix_web::{web, HttpRequest, HttpResponse}; +use dashmap::DashMap; +use futures::TryStreamExt; use itertools::Itertools; use serde::{Deserialize, Serialize}; use sqlx::PgPool; @@ -305,27 +307,14 @@ pub struct ManyUpdateData { pub algorithm: Option, // Defaults to calculation based on size of hash pub hashes: Vec, pub loaders: Option>, - pub loader_fields: Option>>, + pub game_versions: Option>, pub version_types: Option>, } pub async fn update_files( - req: HttpRequest, pool: web::Data, redis: web::Data, update_data: web::Json, - session_queue: web::Data, ) -> Result { - let user_option = get_user_from_headers( - &req, - &**pool, - &redis, - &session_queue, - Some(&[Scopes::VERSION_READ]), - ) - .await - .map(|x| x.1) - .ok(); - let algorithm = update_data .algorithm .clone() @@ -338,16 +327,36 @@ pub async fn update_files( ) .await?; - let projects = database::models::Project::get_many_ids( - &files.iter().map(|x| x.project_id).collect::>(), - &**pool, - &redis, + // TODO: de-hardcode this and actually use version fields system + let update_version_ids = sqlx::query!( + " + SELECT v.id version_id, v.mod_id mod_id + FROM versions v + INNER JOIN version_fields vf ON vf.field_id = 3 AND v.id = vf.version_id + INNER JOIN loader_field_enum_values lfev ON vf.enum_value = lfev.id AND (cardinality($2::varchar[]) = 0 OR lfev.value = ANY($2::varchar[])) + INNER JOIN loaders_versions lv ON lv.version_id = v.id + INNER JOIN loaders l on lv.loader_id = l.id AND (cardinality($3::varchar[]) = 0 OR l.loader = ANY($3::varchar[])) + WHERE v.mod_id = ANY($1) AND (cardinality($4::varchar[]) = 0 OR v.version_type = ANY($4)) + ORDER BY v.date_published ASC + ", + &files.iter().map(|x| x.project_id.0).collect::>(), + &update_data.game_versions.clone().unwrap_or_default(), + &update_data.loaders.clone().unwrap_or_default(), + &update_data.version_types.clone().unwrap_or_default().iter().map(|x| x.to_string()).collect::>(), ) - .await?; - let all_versions = database::models::Version::get_many( - &projects - .iter() - .flat_map(|x| x.versions.clone()) + .fetch(&**pool) + .try_fold(DashMap::new(), |acc : DashMap<_,Vec>, m| { + acc.entry(database::models::ProjectId(m.mod_id)) + .or_default() + .push(database::models::VersionId(m.version_id)); + async move { Ok(acc) } + }) + .await?; + + let versions = database::models::Version::get_many( + &update_version_ids + .into_iter() + .filter_map(|x| x.1.last().copied()) .collect::>(), &**pool, &redis, @@ -355,50 +364,16 @@ pub async fn update_files( .await?; let mut response = HashMap::new(); - - for project in projects { - for file in files.iter().filter(|x| x.project_id == project.inner.id) { - let version = all_versions - .iter() - .filter(|x| x.inner.project_id == file.project_id) - .filter(|x| { - // TODO: Behaviour here is repeated in a few other filtering places, should be abstracted - let mut bool = true; - - if let Some(version_types) = &update_data.version_types { - bool &= version_types - .iter() - .any(|y| y.as_str() == x.inner.version_type); - } - if let Some(loaders) = &update_data.loaders { - bool &= x.loaders.iter().any(|y| loaders.contains(y)); - } - if let Some(loader_fields) = &update_data.loader_fields { - for (key, values) in loader_fields { - bool &= if let Some(x_vf) = - x.version_fields.iter().find(|y| y.field_name == *key) - { - values.iter().any(|v| x_vf.value.contains_json_value(v)) - } else { - true - }; - } - } - - bool - }) - .sorted() - .last(); - - if let Some(version) = version { - if is_visible_version(&version.inner, &user_option, &pool, &redis).await? { - if let Some(hash) = file.hashes.get(&algorithm) { - response.insert( - hash.clone(), - models::projects::Version::from(version.clone()), - ); - } - } + for file in files { + if let Some(version) = versions + .iter() + .find(|x| x.inner.project_id == file.project_id) + { + if let Some(hash) = file.hashes.get(&algorithm) { + response.insert( + hash.clone(), + models::projects::Version::from(version.clone()), + ); } } } diff --git a/tests/common/api_v3/version.rs b/tests/common/api_v3/version.rs index 34ec443f9..005a61e38 100644 --- a/tests/common/api_v3/version.rs +++ b/tests/common/api_v3/version.rs @@ -338,9 +338,7 @@ impl ApiVersion for ApiV3 { json["loaders"] = serde_json::to_value(loaders).unwrap(); } if let Some(game_versions) = game_versions { - json["loader_fields"] = json!({ - "game_versions": game_versions, - }); + json["game_versions"] = serde_json::to_value(game_versions).unwrap(); } if let Some(version_types) = version_types { json["version_types"] = serde_json::to_value(version_types).unwrap();