From ef31c0c0da3a3ac4a3a6a027a3f1a69b8466367e Mon Sep 17 00:00:00 2001 From: Wyatt Verchere Date: Thu, 11 Jan 2024 18:11:27 -0800 Subject: [PATCH] Cleans up and removes TODOs, adds tests (#844) * removes version ordering from v2; simplifies now-unecessary three-level faceting * resolved some todos * test for game version updating * merge fixes; display_categories fix --- src/database/models/legacy_loader_fields.rs | 25 ++----- src/models/v2/projects.rs | 6 -- src/models/v2/search.rs | 3 +- src/routes/v2/projects.rs | 63 +++++------------ src/routes/v2/versions.rs | 3 +- src/routes/v2_reroute.rs | 12 ++-- src/routes/v3/version_creation.rs | 2 - src/routes/v3/version_file.rs | 2 - src/routes/v3/versions.rs | 10 +-- src/search/mod.rs | 8 +-- src/validate/mod.rs | 2 +- tests/common/api_common/models.rs | 3 - tests/common/api_v2/tags.rs | 2 - tests/common/api_v2/team.rs | 27 ++++---- tests/common/api_v3/tags.rs | 1 - tests/common/api_v3/version.rs | 11 ++- tests/common/dummy_data.rs | 69 +++++------------- tests/common/environment.rs | 3 - tests/loader_fields.rs | 77 +++++++++++++++++++++ tests/scopes.rs | 44 +++--------- tests/v2/project.rs | 3 +- tests/v2/scopes.rs | 29 ++++++++ tests/v2/search.rs | 5 +- tests/version.rs | 74 +++++++++++--------- 24 files changed, 243 insertions(+), 241 deletions(-) diff --git a/src/database/models/legacy_loader_fields.rs b/src/database/models/legacy_loader_fields.rs index 80f7e1386..8fbb425d6 100644 --- a/src/database/models/legacy_loader_fields.rs +++ b/src/database/models/legacy_loader_fields.rs @@ -41,15 +41,16 @@ impl MinecraftGameVersion { redis: &RedisPool, ) -> Result, DatabaseError> where - E: sqlx::Executor<'a, Database = sqlx::Postgres> + Copy, + E: sqlx::Acquire<'a, Database = sqlx::Postgres>, { - let game_version_enum = LoaderFieldEnum::get(Self::FIELD_NAME, exec, redis) + let mut exec = exec.acquire().await?; + let game_version_enum = LoaderFieldEnum::get(Self::FIELD_NAME, &mut *exec, redis) .await? .ok_or_else(|| { DatabaseError::SchemaError("Could not find game version enum.".to_string()) })?; let game_version_enum_values = - LoaderFieldEnumValue::list(game_version_enum.id, exec, redis).await?; + LoaderFieldEnumValue::list(game_version_enum.id, &mut *exec, redis).await?; let game_versions = game_version_enum_values .into_iter() @@ -71,24 +72,6 @@ impl MinecraftGameVersion { Ok(game_versions) } - // TODO: remove this - pub async fn list_transaction( - transaction: &mut sqlx::Transaction<'_, sqlx::Postgres>, - redis: &RedisPool, - ) -> Result, DatabaseError> { - let game_version_enum = LoaderFieldEnum::get(Self::FIELD_NAME, &mut **transaction, redis) - .await? - .ok_or_else(|| { - DatabaseError::SchemaError("Could not find game version enum.".to_string()) - })?; - let game_version_enum_values = - LoaderFieldEnumValue::list(game_version_enum.id, &mut **transaction, redis).await?; - Ok(game_version_enum_values - .into_iter() - .map(MinecraftGameVersion::from_enum_value) - .collect()) - } - // Tries to create a MinecraftGameVersion from a VersionField // Clones on success pub fn try_from_version_field( diff --git a/src/models/v2/projects.rs b/src/models/v2/projects.rs index 881f81328..ef9f9acc2 100644 --- a/src/models/v2/projects.rs +++ b/src/models/v2/projects.rs @@ -101,7 +101,6 @@ impl LegacyProject { // Requires any queried versions to be passed in, to get access to certain version fields contained within. // - This can be any version, because the fields are ones that used to be on the project itself. // - Its conceivable that certain V3 projects that have many different ones may not have the same fields on all of them. - // TODO: Should this return an error instead for v2 users? // It's safe to use a db version_item for this as the only info is side types, game versions, and loader fields (for loaders), which used to be public on project anyway. pub fn from(data: Project, versions_item: Option) -> Self { let mut client_side = LegacySideType::Unknown; @@ -289,10 +288,6 @@ pub struct LegacyVersion { /// A list of loaders this project supports (has a newtype struct) pub loaders: Vec, - // TODO: should we remove this? as this is a v3 field and tests for it should be isolated to v3 - // it allows us to keep tests that use this struct in common - pub ordering: Option, - pub id: VersionId, pub project_id: ProjectId, pub author_id: UserId, @@ -354,7 +349,6 @@ impl From for LegacyVersion { files: data.files, dependencies: data.dependencies, game_versions, - ordering: data.ordering, loaders, } } diff --git a/src/models/v2/search.rs b/src/models/v2/search.rs index 5f556afb9..354c0b68f 100644 --- a/src/models/v2/search.rs +++ b/src/models/v2/search.rs @@ -42,7 +42,7 @@ pub struct LegacyResultSearchProject { impl LegacyResultSearchProject { pub fn from(result_search_project: ResultSearchProject) -> Self { let mut categories = result_search_project.categories; - categories.extend(result_search_project.loaders); + categories.extend(result_search_project.loaders.clone()); if categories.contains(&"mrpack".to_string()) { if let Some(mrpack_loaders) = result_search_project .project_loader_fields @@ -58,6 +58,7 @@ impl LegacyResultSearchProject { } } let mut display_categories = result_search_project.display_categories; + display_categories.extend(result_search_project.loaders); if display_categories.contains(&"mrpack".to_string()) { if let Some(mrpack_loaders) = result_search_project .project_loader_fields diff --git a/src/routes/v2/projects.rs b/src/routes/v2/projects.rs index 6dbf1004a..26560d8d1 100644 --- a/src/routes/v2/projects.rs +++ b/src/routes/v2/projects.rs @@ -12,7 +12,6 @@ use crate::routes::v3::projects::ProjectIds; use crate::routes::{v2_reroute, v3, ApiError}; use crate::search::{search_for_project, SearchConfig, SearchError}; use actix_web::{delete, get, patch, post, web, HttpRequest, HttpResponse}; -use itertools::Itertools; use serde::{Deserialize, Serialize}; use sqlx::PgPool; use std::collections::HashMap; @@ -53,35 +52,16 @@ pub async fn project_search( web::Query(info): web::Query, config: web::Data, ) -> Result { - // TODO: make this nicer // Search now uses loader_fields instead of explicit 'client_side' and 'server_side' fields // While the backend for this has changed, it doesnt affect much // in the API calls except that 'versions:x' is now 'game_versions:x' - let facets: Option>>> = if let Some(facets) = info.facets { - let facets = serde_json::from_str::>>(&facets)?; - // Search can now *optionally* have a third inner array: So Vec(AND)>> - // For every inner facet, we will check if it can be deserialized into a Vec<&str>, and do so. - // If not, we will assume it is a single facet and wrap it in a Vec. - let facets: Vec>> = facets - .into_iter() - .map(|facets| { - facets - .into_iter() - .map(|facet| { - if facet.is_array() { - serde_json::from_value::>(facet).unwrap_or_default() - } else { - vec![serde_json::from_value::(facet).unwrap_or_default()] - } - }) - .collect_vec() - }) - .collect_vec(); + let facets: Option>> = if let Some(facets) = info.facets { + let facets = serde_json::from_str::>>(&facets)?; // These loaders specifically used to be combined with 'mod' to be a plugin, but now // they are their own loader type. We will convert 'mod' to 'mod' OR 'plugin' // as it essentially was before. - let facets = v2_reroute::convert_plugin_loaders_v3(facets); + let facets = v2_reroute::convert_plugin_loader_facets_v3(facets); Some( facets @@ -89,27 +69,22 @@ pub async fn project_search( .map(|facet| { facet .into_iter() - .map(|facets| { - facets - .into_iter() - .map(|facet| { - if let Some((key, operator, val)) = parse_facet(&facet) { - format!( - "{}{}{}", - match key.as_str() { - "versions" => "game_versions", - "project_type" => "project_types", - "title" => "name", - x => x, - }, - operator, - val - ) - } else { - facet.to_string() - } - }) - .collect::>() + .map(|facet| { + if let Some((key, operator, val)) = parse_facet(&facet) { + format!( + "{}{}{}", + match key.as_str() { + "versions" => "game_versions", + "project_type" => "project_types", + "title" => "name", + x => x, + }, + operator, + val + ) + } else { + facet.to_string() + } }) .collect::>() }) diff --git a/src/routes/v2/versions.rs b/src/routes/v2/versions.rs index 85e1c7fbf..56aeb6a2e 100644 --- a/src/routes/v2/versions.rs +++ b/src/routes/v2/versions.rs @@ -215,7 +215,6 @@ pub struct EditVersion { pub downloads: Option, pub status: Option, pub file_types: Option>, - pub ordering: Option>, //TODO: How do you actually pass this in json? } #[derive(Serialize, Deserialize)] @@ -291,7 +290,7 @@ pub async fn version_edit( }) .collect::>() }), - ordering: new_version.ordering, + ordering: None, fields, }; diff --git a/src/routes/v2_reroute.rs b/src/routes/v2_reroute.rs index dae4f4101..e2146ab80 100644 --- a/src/routes/v2_reroute.rs +++ b/src/routes/v2_reroute.rs @@ -178,18 +178,18 @@ pub fn convert_side_types_v3( fields } -// Converts plugin loaders from v2 to v3 +// Converts plugin loaders from v2 to v3, for search facets // Within every 1st and 2nd level (the ones allowed in v2), we convert every instance of: // "project_type:mod" to "project_type:plugin" OR "project_type:mod" -pub fn convert_plugin_loaders_v3(facets: Vec>>) -> Vec>> { +pub fn convert_plugin_loader_facets_v3(facets: Vec>) -> Vec> { facets .into_iter() .map(|inner_facets| { - if inner_facets == [["project_type:mod"]] { + if inner_facets == ["project_type:mod"] { vec![ - vec!["project_type:plugin".to_string()], - vec!["project_type:datapack".to_string()], - vec!["project_type:mod".to_string()], + "project_type:plugin".to_string(), + "project_type:datapack".to_string(), + "project_type:mod".to_string(), ] } else { inner_facets diff --git a/src/routes/v3/version_creation.rs b/src/routes/v3/version_creation.rs index f04fbe976..6bbd7e83e 100644 --- a/src/routes/v3/version_creation.rs +++ b/src/routes/v3/version_creation.rs @@ -583,8 +583,6 @@ async fn upload_file_to_version_inner( }; let all_loaders = models::loader_fields::Loader::list(&mut **transaction, &redis).await?; - - // TODO: this coded is reused a lot, it should be refactored into a function let selected_loaders = version .loaders .iter() diff --git a/src/routes/v3/version_file.rs b/src/routes/v3/version_file.rs index 2c2cbf655..8d3885c82 100644 --- a/src/routes/v3/version_file.rs +++ b/src/routes/v3/version_file.rs @@ -132,7 +132,6 @@ pub async fn get_update_from_hash( .map(|x| x.1) .ok(); let hash = info.into_inner().0.to_lowercase(); - if let Some(file) = database::models::Version::get_file_from_hash( hash_query .algorithm @@ -185,7 +184,6 @@ pub async fn get_update_from_hash( } } } - Err(ApiError::NotFound) } diff --git a/src/routes/v3/versions.rs b/src/routes/v3/versions.rs index 01c80929b..ff090e343 100644 --- a/src/routes/v3/versions.rs +++ b/src/routes/v3/versions.rs @@ -202,8 +202,12 @@ pub struct EditVersion { pub downloads: Option, pub status: Option, pub file_types: Option>, - - pub ordering: Option>, //TODO: How do you actually pass this in json? + #[serde( + default, + skip_serializing_if = "Option::is_none", + with = "::serde_with::rust::double_option" + )] + pub ordering: Option>, // Flattened loader fields // All other fields are loader-specific VersionFields @@ -220,8 +224,6 @@ pub struct EditVersionFileType { pub file_type: Option, } -// TODO: Avoid this 'helper' pattern here and similar fnunctoins- a macro might be the best bet here to ensure it's callable from both v2 and v3 -// (web::Path can't be recreated naturally) pub async fn version_edit( req: HttpRequest, info: web::Path<(VersionId,)>, diff --git a/src/search/mod.rs b/src/search/mod.rs index 5c64b3c55..72308538c 100644 --- a/src/search/mod.rs +++ b/src/search/mod.rs @@ -131,8 +131,8 @@ pub struct UploadSearchProject { pub loaders: Vec, // Search uses loaders as categories- this is purely for the Project model. pub links: Vec, pub gallery_items: Vec, // Gallery *only* urls are stored in gallery, but the gallery items are stored here- required for the Project model. - pub games: Vec, // Todo: in future, could be a searchable field. - pub organization_id: Option, // Todo: in future, could be a searchable field. + pub games: Vec, + pub organization_id: Option, pub project_loader_fields: HashMap>, // Aggregation of loader_fields from all versions of the project, allowing for reconstruction of the Project model. #[serde(flatten)] @@ -183,8 +183,8 @@ pub struct ResultSearchProject { pub loaders: Vec, // Search uses loaders as categories- this is purely for the Project model. pub links: Vec, pub gallery_items: Vec, // Gallery *only* urls are stored in gallery, but the gallery items are stored here- required for the Project model. - pub games: Vec, // Todo: in future, could be a searchable field. - pub organization_id: Option, // Todo: in future, could be a searchable field. + pub games: Vec, + pub organization_id: Option, pub project_loader_fields: HashMap>, // Aggregation of loader_fields from all versions of the project, allowing for reconstruction of the Project model. #[serde(flatten)] diff --git a/src/validate/mod.rs b/src/validate/mod.rs index d305b5752..6254e4cf4 100644 --- a/src/validate/mod.rs +++ b/src/validate/mod.rs @@ -132,7 +132,7 @@ pub async fn validate_file( .find_map(|v| MinecraftGameVersion::try_from_version_field(&v).ok()) .unwrap_or_default(); let all_game_versions = - MinecraftGameVersion::list_transaction(&mut *transaction, redis).await?; + MinecraftGameVersion::list(None, None, &mut *transaction, redis).await?; validate_minecraft_file( data, file_extension, diff --git a/tests/common/api_common/models.rs b/tests/common/api_common/models.rs index acfd3042e..f756a12a1 100644 --- a/tests/common/api_common/models.rs +++ b/tests/common/api_common/models.rs @@ -77,9 +77,6 @@ pub struct CommonVersion { pub requested_status: Option, pub files: Vec, pub dependencies: Vec, - - // TODO: should ordering be in v2? - pub ordering: Option, } #[derive(Deserialize)] diff --git a/tests/common/api_v2/tags.rs b/tests/common/api_v2/tags.rs index 75f70b2ab..2749e5cad 100644 --- a/tests/common/api_v2/tags.rs +++ b/tests/common/api_v2/tags.rs @@ -21,8 +21,6 @@ use crate::{ use super::ApiV2; -// TODO: Tag gets do not include PAT, as they are public. - impl ApiV2 { async fn get_side_types(&self) -> ServiceResponse { let req = TestRequest::get() diff --git a/tests/common/api_v2/team.rs b/tests/common/api_v2/team.rs index 0ea7ba3f2..015cf34dd 100644 --- a/tests/common/api_v2/team.rs +++ b/tests/common/api_v2/team.rs @@ -66,10 +66,11 @@ impl ApiTeams for ApiV2 { ) -> Vec { let resp = self.get_team_members(id_or_title, pat).await; assert_status!(&resp, StatusCode::OK); - // TODO: Note, this does NOT deserialize to any other struct first, as currently TeamMember is the same in v2 and v3. - // CommonTeamMember = TeamMember (v3) - // This may yet change, so we should keep common struct. - test::read_body_json(resp).await + // First, deserialize to the non-common format (to test the response is valid for this api version) + let v: Vec = test::read_body_json(resp).await; + // Then, deserialize to the common format + let value = serde_json::to_value(v).unwrap(); + serde_json::from_value(value).unwrap() } async fn get_teams_members( @@ -103,10 +104,11 @@ impl ApiTeams for ApiV2 { ) -> Vec { let resp = self.get_project_members(id_or_title, pat).await; assert_status!(&resp, StatusCode::OK); - // TODO: Note, this does NOT deserialize to any other struct first, as currently TeamMember is the same in v2 and v3. - // CommonTeamMember = TeamMember (v3) - // This may yet change, so we should keep common struct. - test::read_body_json(resp).await + // First, deserialize to the non-common format (to test the response is valid for this api version) + let v: Vec = test::read_body_json(resp).await; + // Then, deserialize to the common format + let value = serde_json::to_value(v).unwrap(); + serde_json::from_value(value).unwrap() } async fn get_organization_members( @@ -128,10 +130,11 @@ impl ApiTeams for ApiV2 { ) -> Vec { let resp = self.get_organization_members(id_or_title, pat).await; assert_status!(&resp, StatusCode::OK); - // TODO: Note, this does NOT deserialize to any other struct first, as currently TeamMember is the same in v2 and v3. - // CommonTeamMember = TeamMember (v3) - // This may yet change, so we should keep common struct. - test::read_body_json(resp).await + // First, deserialize to the non-common format (to test the response is valid for this api version) + let v: Vec = test::read_body_json(resp).await; + // Then, deserialize to the common format + let value = serde_json::to_value(v).unwrap(); + serde_json::from_value(value).unwrap() } async fn join_team(&self, team_id: &str, pat: Option<&str>) -> ServiceResponse { diff --git a/tests/common/api_v3/tags.rs b/tests/common/api_v3/tags.rs index fccc2d748..679f519a1 100644 --- a/tests/common/api_v3/tags.rs +++ b/tests/common/api_v3/tags.rs @@ -85,7 +85,6 @@ impl ApiV3 { test::read_body_json(resp).await } - // TODO: fold this into v3 API of other v3 testing PR async fn get_games(&self) -> ServiceResponse { let req = TestRequest::get() .uri("/v3/games") diff --git a/tests/common/api_v3/version.rs b/tests/common/api_v3/version.rs index 688e61e07..34ec443f9 100644 --- a/tests/common/api_v3/version.rs +++ b/tests/common/api_v3/version.rs @@ -66,6 +66,16 @@ impl ApiV3 { test::read_body_json(resp).await } + pub async fn get_versions_deserialized( + &self, + version_ids: Vec, + pat: Option<&str>, + ) -> Vec { + let resp = self.get_versions(version_ids, pat).await; + assert_status!(&resp, StatusCode::OK); + test::read_body_json(resp).await + } + pub async fn update_individual_files( &self, algorithm: &str, @@ -454,7 +464,6 @@ impl ApiVersion for ApiV3 { serde_json::from_value(value).unwrap() } - // TODO: remove redundancy in these functions async fn edit_version_ordering( &self, version_id: &str, diff --git a/tests/common/dummy_data.rs b/tests/common/dummy_data.rs index 6b3843b2b..b2cdcb0c5 100644 --- a/tests/common/dummy_data.rs +++ b/tests/common/dummy_data.rs @@ -15,9 +15,8 @@ use zip::{write::FileOptions, CompressionMethod, ZipWriter}; use crate::{ assert_status, - common::{api_common::Api, database::USER_USER_PAT}, + common::{api_common::Api, api_v3, database::USER_USER_PAT}, }; -use labrinth::util::actix::{AppendsMultipart, MultipartSegment, MultipartSegmentData}; use super::{ api_common::{request_data::ImageData, ApiProject, AppendsOptionalPat}, @@ -342,58 +341,22 @@ pub async fn add_project_beta(api: &ApiV3) -> (Project, Version) { // Generate test project data. let jar = TestFile::DummyProjectBeta; // TODO: this shouldnt be hardcoded (nor should other similar ones be) - let json_data = json!( - { - "name": "Test Project Beta", - "slug": "beta", - "summary": "A dummy project for testing with.", - "description": "This project is not-yet-approved, and versions are draft.", - "initial_versions": [{ - "file_parts": [jar.filename()], - "version_number": "1.2.3", - "version_title": "start", - "status": "unlisted", - "dependencies": [], - "singleplayer": true, - "client_and_server": true, - "client_only": true, - "server_only": false, - "game_versions": ["1.20.1"] , - "release_channel": "release", - "loaders": ["fabric"], - "featured": true - }], - "status": "private", - "requested_status": "private", - "categories": [], - "license_id": "MIT" - } + + let modify_json = serde_json::from_value(json!([ + { "op": "add", "path": "/summary", "value": "A dummy project for testing with." }, + { "op": "add", "path": "/description", "value": "This project is not-yet-approved, and versions are draft." }, + { "op": "add", "path": "/initial_versions/0/status", "value": "unlisted" }, + { "op": "add", "path": "/status", "value": "private" }, + { "op": "add", "path": "/requested_status", "value": "private" }, + ])) + .unwrap(); + + let creation_data = api_v3::request_data::get_public_project_creation_data( + "beta", + Some(jar), + Some(modify_json), ); - - // Basic json - let json_segment = MultipartSegment { - name: "data".to_string(), - filename: None, - content_type: Some("application/json".to_string()), - data: MultipartSegmentData::Text(serde_json::to_string(&json_data).unwrap()), - }; - - // Basic file - let file_segment = MultipartSegment { - name: jar.filename(), - filename: Some(jar.filename()), - content_type: Some("application/java-archive".to_string()), - data: MultipartSegmentData::Binary(jar.bytes()), - }; - - // Add a project. - let req = TestRequest::post() - .uri("/v3/project") - .append_pat(USER_USER_PAT) - .set_multipart(vec![json_segment.clone(), file_segment.clone()]) - .to_request(); - let resp = api.call(req).await; - assert_status!(&resp, StatusCode::OK); + api.create_project(creation_data, USER_USER_PAT).await; get_project_beta(api).await } diff --git a/tests/common/environment.rs b/tests/common/environment.rs index e83e68111..8ac0f8c78 100644 --- a/tests/common/environment.rs +++ b/tests/common/environment.rs @@ -25,9 +25,6 @@ pub async fn with_test_environment( db.cleanup().await; } -// TODO: This needs to be slightly redesigned in order to do both V2 and v3 tests. -// TODO: Most tests, since they use API functions, can be applied to both. The ones that weren't are in v2/, but -// all tests that can be applied to both should use both v2 and v3 (extract api to a trait with all the API functions and call both). pub async fn with_test_environment_all(max_connections: Option, f: F) where Fut: Future, diff --git a/tests/loader_fields.rs b/tests/loader_fields.rs index 53ca5f858..c6d3f16eb 100644 --- a/tests/loader_fields.rs +++ b/tests/loader_fields.rs @@ -5,6 +5,7 @@ use actix_web::test; use common::api_v3::ApiV3; use common::environment::{with_test_environment, TestEnvironment}; use itertools::Itertools; +use labrinth::database::models::legacy_loader_fields::MinecraftGameVersion; use labrinth::models::v3; use serde_json::json; @@ -556,3 +557,79 @@ async fn test_multi_get_redis_cache() { }) .await; } + +#[actix_rt::test] +async fn minecraft_game_version_update() { + // We simulate adding a Minecraft game version, to ensure other data doesn't get overwritten + // This is basically a test for the insertion/concatenation query + // This doesn't use a route (as this behaviour isn't exposed via a route, but a scheduled URL call) + // We just interact with the labrinth functions directly + with_test_environment(None, |test_env: TestEnvironment| async move { + let api = &test_env.api; + + // First, get a list of all gameversions + let game_versions = api + .get_loader_field_variants_deserialized("game_versions") + .await; + + // A couple specific checks- in the dummy data, all game versions are marked as major=false except 1.20.5 + let name_to_major = game_versions + .iter() + .map(|x| { + ( + x.value.clone(), + x.metadata.get("major").unwrap().as_bool().unwrap(), + ) + }) + .collect::>(); + for (name, major) in name_to_major { + if name == "1.20.5" { + assert!(major); + } else { + assert!(!major); + } + } + + // Now, we add a new game version, directly to the db + let pool = test_env.db.pool.clone(); + let redis = test_env.db.redis_pool.clone(); + MinecraftGameVersion::builder() + .version("1.20.6") + .unwrap() + .version_type("release") + .unwrap() + .created( + // now + &chrono::Utc::now(), + ) + .insert(&pool, &redis) + .await + .unwrap(); + + // Check again + let game_versions = api + .get_loader_field_variants_deserialized("game_versions") + .await; + + let name_to_major = game_versions + .iter() + .map(|x| { + ( + x.value.clone(), + x.metadata.get("major").unwrap().as_bool().unwrap(), + ) + }) + .collect::>(); + // Confirm that the new version is there + assert!(name_to_major.contains_key("1.20.6")); + // Confirm metadata is unaltered + for (name, major) in name_to_major { + if name == "1.20.5" { + assert!(major); + } else { + assert!(!major); + } + } + }) + .await +} diff --git a/tests/scopes.rs b/tests/scopes.rs index 9a6013b5d..7ebc637be 100644 --- a/tests/scopes.rs +++ b/tests/scopes.rs @@ -218,7 +218,6 @@ pub async fn notifications_scopes() { #[actix_rt::test] pub async fn project_version_create_scopes_v3() { with_test_environment(None, |test_env: TestEnvironment| async move { - // TODO: If possible, find a way to use generic api functions with the Permissions/Scopes test, then this can be recombined with the V2 version of this test let api = &test_env.api; // Create project @@ -409,16 +408,14 @@ pub async fn project_version_reads_scopes() { .await .unwrap(); - // TODO: Should this be /POST? Looks like /GET - // TODO: this scope doesn't actually affect anything, because the Project::get_id contained within disallows hidden versions, which is the point of this scope - // let req_gen = || { - // test::TestRequest::post() - // .uri(&format!("/v3/version_file/{beta_file_hash}/update")) - // .set_json(json!({})) + // TODO: This scope currently fails still as the 'version' field of QueryProject only allows public versions. + // TODO: This will be fixed when the 'extracts_versions' PR is merged. + // let req_gen = |pat: Option| async move { + // api.get_update_from_hash(beta_file_hash, "sha1", None, None, None, pat.as_deref()) + // .await // }; // ScopeTest::new(&test_env).with_failure_code(404).test(req_gen, read_version).await.unwrap(); - // TODO: Should this be /POST? Looks like /GET let req_gen = |pat: Option| async move { api.get_versions_from_hashes(&[beta_file_hash], "sha1", pat.as_deref()) .await @@ -432,30 +429,10 @@ pub async fn project_version_reads_scopes() { assert!(success.as_object().unwrap().contains_key(beta_file_hash)); // Update version file - // TODO: Should this be /POST? Looks like /GET - // TODO: this scope doesn't actually affect anything, because the Project::get_id contained within disallows hidden versions, which is the point of this scope - - // let req_gen = || { - // test::TestRequest::post() - // .uri(&format!("/v3/version_files/update_individual")) - // .set_json(json!({ - // "hashes": [{ - // "hash": beta_file_hash, - // }] - // })) - // }; - // let (failure, success) = ScopeTest::new(&test_env).with_failure_code(200).test(req_gen, read_version).await.unwrap(); - // assert!(!failure.as_object().unwrap().contains_key(beta_file_hash)); - // assert!(success.as_object().unwrap().contains_key(beta_file_hash)); - - // Update version file - // TODO: this scope doesn't actually affect anything, because the Project::get_id contained within disallows hidden versions, which is the point of this scope - // let req_gen = || { - // test::TestRequest::post() - // .uri(&format!("/v3/version_files/update")) - // .set_json(json!({ - // "hashes": [beta_file_hash] - // })) + // TODO: This scope currently fails still as the 'version' field of QueryProject only allows public versions. + // TODO: This will be fixed when the 'extracts_versions' PR is merged. + // let req_gen = |pat : Option| async move { + // api.update_files("sha1", vec![beta_file_hash.clone()], None, None, None, pat.as_deref()).await // }; // let (failure, success) = ScopeTest::new(&test_env).with_failure_code(200).test(req_gen, read_version).await.unwrap(); // assert!(!failure.as_object().unwrap().contains_key(beta_file_hash)); @@ -712,8 +689,7 @@ pub async fn version_write_scopes() { .await .unwrap(); - // Delete version file - // TODO: Should this scope be VERSION_DELETE? + // Delete version file. Notably, this uses 'VERSION_WRITE' instead of 'VERSION_DELETE' as it is writing to the version let req_gen = |pat: Option| async move { api.remove_version_file(alpha_file_hash, pat.as_deref()) .await diff --git a/tests/v2/project.rs b/tests/v2/project.rs index 57e8b7817..1352ee1f6 100644 --- a/tests/v2/project.rs +++ b/tests/v2/project.rs @@ -90,7 +90,7 @@ async fn test_project_type_sanity() { ); } - // TODO: as we get more complicated strucures with v3 testing, and alpha/beta get more complicated, we should add more tests here, + // As we get more complicated strucures with as v3 continues to expand, and alpha/beta get more complicated, we should add more tests here, // to ensure that projects created with v3 routes are still valid and work with v3 routes. }) .await; @@ -397,7 +397,6 @@ async fn permissions_patch_project_v2() { with_test_environment(Some(8), |test_env: TestEnvironment| async move { let api = &test_env.api; - // TODO: This only includes v2 ones (as it should. See v3) // For each permission covered by EDIT_DETAILS, ensure the permission is required let edit_details = ProjectPermissions::EDIT_DETAILS; let test_pairs = [ diff --git a/tests/v2/scopes.rs b/tests/v2/scopes.rs index a0e3324f8..cac103e61 100644 --- a/tests/v2/scopes.rs +++ b/tests/v2/scopes.rs @@ -50,3 +50,32 @@ pub async fn project_version_create_scopes() { }) .await; } + +#[actix_rt::test] +pub async fn project_version_reads_scopes() { + with_test_environment(None, |_test_env: TestEnvironment| async move { + // let api = &test_env.api; + // let beta_file_hash = &test_env.dummy.project_beta.file_hash; + + // let read_version = Scopes::VERSION_READ; + + // Update individual version file + // TODO: This scope currently fails still as the 'version' field of QueryProject only allows public versions. + // TODO: This will be fixed when the 'extracts_versions' PR is merged. + // let req_gen = |pat : Option| async move { + // api.update_individual_files("sha1", vec![ + // FileUpdateData { + // hash: beta_file_hash.clone(), + // loaders: None, + // game_versions: None, + // version_types: None + // } + // ], pat.as_deref()) + // .await + // }; + // let (failure, success) = ScopeTest::new(&test_env).with_failure_code(200).test(req_gen, read_version).await.unwrap(); + // assert!(!failure.as_object().unwrap().contains_key(beta_file_hash)); + // assert!(success.as_object().unwrap().contains_key(beta_file_hash)); + }) + .await; +} diff --git a/tests/v2/search.rs b/tests/v2/search.rs index fbad4fd94..4d3db3683 100644 --- a/tests/v2/search.rs +++ b/tests/v2/search.rs @@ -18,9 +18,6 @@ use std::sync::Arc; #[actix_rt::test] async fn search_projects() { - // TODO: ("Match changes in the 2 version of thee add_public_version_creation_data to those made in v3 - // It should drastically simplify this function - // Test setup and dummy data with_test_environment(Some(10), |test_env: TestEnvironment| async move { let api = &test_env.api; @@ -399,6 +396,8 @@ async fn search_projects() { ); assert!(hit.categories.contains(&"forge".to_string())); assert!(hit.categories.contains(&"fabric".to_string())); + assert!(hit.display_categories.contains(&"forge".to_string())); + assert!(hit.display_categories.contains(&"fabric".to_string())); // Also, ensure author is correctly capitalized assert_eq!(hit.author, "User".to_string()); diff --git a/tests/version.rs b/tests/version.rs index d7dd803da..de5878316 100644 --- a/tests/version.rs +++ b/tests/version.rs @@ -536,49 +536,55 @@ pub async fn test_project_versions() { #[actix_rt::test] async fn can_create_version_with_ordering() { - with_test_environment_all(None, |env| async move { - let alpha_project_id_parsed = env.dummy.project_alpha.project_id_parsed; + with_test_environment( + None, + |env: common::environment::TestEnvironment| async move { + let alpha_project_id_parsed = env.dummy.project_alpha.project_id_parsed; - let new_version_id = get_json_val_str( - env.api - .add_public_version_deserialized_common( - alpha_project_id_parsed, - "1.2.3.4", - TestFile::BasicMod, - Some(1), - None, - USER_USER_PAT, - ) - .await - .id, - ); + let new_version_id = get_json_val_str( + env.api + .add_public_version_deserialized_common( + alpha_project_id_parsed, + "1.2.3.4", + TestFile::BasicMod, + Some(1), + None, + USER_USER_PAT, + ) + .await + .id, + ); - let versions = env - .api - .get_versions_deserialized_common(vec![new_version_id.clone()], USER_USER_PAT) - .await; - assert_eq!(versions[0].ordering, Some(1)); - }) + let versions = env + .api + .get_versions_deserialized(vec![new_version_id.clone()], USER_USER_PAT) + .await; + assert_eq!(versions[0].ordering, Some(1)); + }, + ) .await; } #[actix_rt::test] async fn edit_version_ordering_works() { - with_test_environment_all(None, |env| async move { - let alpha_version_id = env.dummy.project_alpha.version_id.clone(); + with_test_environment( + None, + |env: common::environment::TestEnvironment| async move { + let alpha_version_id = env.dummy.project_alpha.version_id.clone(); - let resp = env - .api - .edit_version_ordering(&alpha_version_id, Some(10), USER_USER_PAT) - .await; - assert_status!(&resp, StatusCode::NO_CONTENT); + let resp = env + .api + .edit_version_ordering(&alpha_version_id, Some(10), USER_USER_PAT) + .await; + assert_status!(&resp, StatusCode::NO_CONTENT); - let versions = env - .api - .get_versions_deserialized_common(vec![alpha_version_id.clone()], USER_USER_PAT) - .await; - assert_eq!(versions[0].ordering, Some(10)); - }) + let versions = env + .api + .get_versions_deserialized(vec![alpha_version_id.clone()], USER_USER_PAT) + .await; + assert_eq!(versions[0].ordering, Some(10)); + }, + ) .await; }